Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FetchRequest name #31

Merged
merged 6 commits into from
Jul 23, 2021
Merged

FetchRequest name #31

merged 6 commits into from
Jul 23, 2021

Conversation

lickel
Copy link
Collaborator

@lickel lickel commented Jul 23, 2021

Rename FetchRequest => FetchDefinition

@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2021

Codecov Report

❗ No coverage uploaded for pull request base (v4-development@2f56607). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                @@
##             v4-development      #31   +/-   ##
=================================================
  Coverage                  ?   93.66%           
=================================================
  Files                     ?       30           
  Lines                     ?     4861           
  Branches                  ?        0           
=================================================
  Hits                      ?     4553           
  Misses                    ?      308           
  Partials                  ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f56607...d0f2bba. Read the comment docs.

Copy link
Collaborator

@AdamPro13 AdamPro13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First bike shed

@@ -13,7 +13,7 @@ import FetchRequests
class ViewController: UITableViewController {
private(set) lazy var controller: FetchedResultsController<Model> = {
let controller: FetchedResultsController<Model> = FetchedResultsController(
request: Model.fetchRequest(),
fetchDefinition: Model.fetchDefinition(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would making the signature definition instead of fetchDefinition be cleaner? It's in the context of the FetchedResultsController, so I think the fetch can be implied.

Copy link
Collaborator

@AdamPro13 AdamPro13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lickel lickel merged commit a0b93bb into v4-development Jul 23, 2021
@lickel lickel deleted the fetch-name branch July 23, 2021 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants