Adds decrement() to ParseObject #1069
Conversation
updated master
Add support to add hint to Parse.Query (parse-community#1054)
…-SDK-JS into metadata-tags
Codecov Report
@@ Coverage Diff @@
## master #1069 +/- ##
=======================================
Coverage 92.27% 92.27%
=======================================
Files 54 54
Lines 5215 5215
Branches 1169 1169
=======================================
Hits 4812 4812
Misses 403 403Continue to review full report at Codecov.
|
package.json
Outdated
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "name": "parse", | |||
| "name": "@leapllc/parse", | |||
There was a problem hiding this comment.
doh is right... I accidentally committed this
|
My take is that this is duplicate code for no added value. it also isn't available in any of our other apis or rest. I appreciate the effort @stevestencil. |
|
If @davimacedo, @dplewis, and/or @TomWFox want to include this change, I at least think we should change the implementation to call increment under the covers. |
|
I'm not completely against the change but agree it should call increment otherwise it's just more code to maintain run tests on - even if it is simple. Not sure about other api references / guides but the JS API Reference does not mention that a negative number can be used (for increment) - it would be nice to mention that if this change doesn't go ahead. |
|
I initially was going to use the increment() function under the hood but the error message that is thrown would have to change too... that's why I did it this way |
|
I am not against of having this code here as well. @TomWFox what does our governance say in the case nobody is against? :) |
|
@davimacedo Considering we can’t reach a lazy consensus we should take a vote, and if it’s a draw then I suggest we include other contributors in the vote. If you are a PMC member, react to this comment with a thumbs up (👍) to vote in favour of merging this PR (in its current state) and a thumbs down (👎) to vote against - if you would vote in favour with changes being made please explain. |
|
Other members of the community are welcome to vote / express an opinion but at this stage only the votes of PMC members are binding. |
|
I'll vote for it but some considerations.
For a feature like this is it worth it to update every SDK, API Reference and Guide? I can help update the other repos. |
|
that would be the same as object.increment('field', 2) |
|
Making it positive input only removes unforeseen events like that. I can only assume parse never had a decrement function because it is based off MongoDB. |
|
I see how it can be confusing but the same could be said about object.increment('field', -1) You make a good point about MongoDB not having the decrement option. What made me submit this PR was I needed to decrement a field and assumed since we had increment(), we would also have decrement(). I was actually surprised when I didn't see it in the docs (same with File.destroy() missing). I think a good argument for having this in here is we have a way to increment a field by 1 by just calling object.increment('field'), but we do not have a way to decrement a field without also having to pass in -1. This PR adds that ability. |
|
@acinader @davimacedo Can you have a look at the discussion and vote? |
|
Just voted. I don't see so much upside on merging but almost no downside as well. I think it is ok to have it in the SDK. |
|
Davi's comment is convincing to me. I don't think we need to update the guide or worry about the other SDKs as this is a minor variation and there are other minor variations. I appreciate the initiative of @stevestencil |
|
@stevestencil Thanks for the PR. Sorry it took so long to get in. |
No description provided.