Skip to content

Conversation

@BufferUnderflower
Copy link
Contributor

@dplewis
Copy link
Member

dplewis commented Jul 21, 2019

Can you add a disclaimer? Count operations can be slow and expensive.

@dplewis
Copy link
Member

dplewis commented Jul 21, 2019

Looks good, I’ll add this to the PHP SDK as well.

@dplewis dplewis requested a review from TomWFox July 21, 2019 13:53
Copy link
Contributor

@TomWFox TomWFox left a comment

Choose a reason for hiding this comment

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

Thanks for contributing, just a couple minor suggestions.


If you want to know the total number of rows in a table satisfying your query, for e.g. pagination purposes - you can use `withCount` (`false` by default).

**Note:** Enabling this flag will change the structure of response, see the example below.
Copy link
Member

@dplewis dplewis Jul 21, 2019

Choose a reason for hiding this comment

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

I thought it was true by default? You can just use query.withCount() no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want to know the total number of rows in a table satisfying your query, for e.g. pagination purposes - you can use withCount() (optionally accepts a boolean, defaults to true when called without arguments or undefined).

Will this be more clear?

Copy link
Member

Choose a reason for hiding this comment

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

(optionally accepts a boolean, defaults to true) should be fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can't this be mis-interpreted like it's an always enabled by default on every query?

Copy link
Member

Choose a reason for hiding this comment

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

@TomWFox What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @dplewis, it's referencing that particular method not querying in general

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, you could not mention the optional boolean as its not strictly relevant to this explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, jsdoc will show these details anyway when it comes to coding

Copy link
Contributor

Choose a reason for hiding this comment

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

@dplewis are you happy with that change?

@dplewis dplewis merged commit 4973012 into parse-community:gh-pages Jul 23, 2019
@BufferUnderflower BufferUnderflower deleted the BufferUnderflower-withCount branch July 23, 2019 12:41
@dplewis
Copy link
Member

dplewis commented Jul 23, 2019

Thanks for contributing! I added this to the PHP SDK too.

@BufferUnderflower
Copy link
Contributor Author

image
@dplewis Seems styling for markdown quotes is a bit too much

@dplewis
Copy link
Member

dplewis commented Jul 23, 2019

Nice catch! I can take a look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants