Skip to content

add includeKeys convenience to PFQuery #765

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 20, 2016
Merged

add includeKeys convenience to PFQuery #765

merged 1 commit into from
Jan 20, 2016

Conversation

AndrewSB
Copy link
Contributor

Thought this was a useful method

@nlutsenko
Copy link
Contributor

I think it is, indeed.
Let's do a little bit of work to improve it then...
Pieces that would be required to get this merged in:

  • Adding the method to public header file
  • Adding documentation on the method in the public header
  • Specifying the generic type of the NSArray * to be NSArray<NSString *> *, since it adds more type safety.
  • Adding includeKeys: method to PFMutableQueryState and using it instead of calling includeKey: in a loop, to slightly improve performance (this is nitpicking)

Let me know if you need help with any of these.

@AndrewSB
Copy link
Contributor Author

Sorry about not thinking to do all of that myself 😬.

Haven't done much Objective-C in about 18 months. I'll make those changes in the next couple days and ping you when I think it's ready to go 👌

@AndrewSB
Copy link
Contributor Author

@nlutsenko would you like me to also squash all of my commits?

@nlutsenko
Copy link
Contributor

Yes, please.

nlutsenko
nlutsenko reviewed Jan 19, 2016
View reviewed changes
Parse/PFQuery.h Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: PFObjects

@nlutsenko
Copy link
Contributor

A single nit, otherwise looks great!
Squash - yes, please, if it's not a problem.

AndrewSB
AndrewSB reviewed Jan 20, 2016
View reviewed changes
Parse/PFQuery.h
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a "`" to the docs for includeKey as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch, thank you!

@AndrewSB
add includeKeys convenience to PFQuery
72b18ff 
Thought this was a useful method

add NSString type annotation + includeKeys to PFQuery header

add includeKeys:NSArray<NSString> to PFMutableQueryState

update PFQuery includeKeys to use internal state method

update NSString => NSString *

PFObjects -> `PFObject`s
@nlutsenko nlutsenko self-assigned this Jan 20, 2016
@nlutsenko nlutsenko added this to the 1.13.0 milestone Jan 20, 2016
nlutsenko added a commit that referenced this pull request Jan 20, 2016
@nlutsenko
Merge pull request #765 from AndrewSB/patch-1
a4eb075 
add includeKeys convenience to PFQuery
@nlutsenko nlutsenko merged commit a4eb075 into parse-community:master Jan 20, 2016
@AndrewSB AndrewSB deleted the patch-1 branch January 20, 2016 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.
3 participants
@AndrewSB @nlutsenko @facebook-github-bot