-
-
Notifications
You must be signed in to change notification settings - Fork 735
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
feat: add support for custom objectId #1088
feat: add support for custom objectId #1088
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@martinpfannemueller I have reviewed your code and looks quite clean and nice to me. Can you merge master here in your branch and check the small feedback I left.
Thanks
Looking at the changes, should we add a lint step to the CI? |
Please open an issue @mtrezza I can take it |
Great, there it is! #1119 |
…-Android into feature/custom_objectId # Conflicts: # parse/src/test/java/com/parse/ParseClientConfigurationTest.java # parse/src/test/java/com/parse/ParseObjectTest.java
I will reformat the title to use the proper commit message syntax. |
Thanks for opening this pull request!
|
I have no idea why the two tests testObjectNotFoundWhenSaveAsync and testMissingRequiredFieldWhenSaveAsync in the ParseInstallationTest-file fail here. Locally everything runs just fine. Attached you will find the output of running the task jacocoTestReport locally. |
@martinpfannemueller can you close and re-open the PR so we can trigger another CI |
@mtrezza how we can re-run the CI build. I tried already closing and re-opening the issue. |
I suggest to first resolve the conflicts, then we look into the CI. |
## [2.0.1](parse-community/Parse-SDK-Android@2.0.0...2.0.1) (2021-10-14) ### Bug Fixes * add maven publications to configure the Jitpack releases ([parse-community#1128](parse-community#1128)) ([67c4fb6](parse-community@67c4fb6))
## [2.0.1](mtrezza/Parse-SDK-Android@2.0.0...2.0.1) (2021-10-14) ### Bug Fixes * add maven publications to configure the Jitpack releases ([parse-community#1128](https://github.com/mtrezza/Parse-SDK-Android/issues/1128)) ([67c4fb6](mtrezza@67c4fb6))
…-Android into feature/custom_objectId # Conflicts: # parse/src/main/java/com/parse/ParseRESTObjectCommand.java # parse/src/test/java/com/parse/ParseObjectTest.java
I added an explicit test for the ParseRESTCommand and the logic of it. The test coverage should be fine now. |
Running tests... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please also add doc headers to the new public properties and methods? So that this feature appears in the API documentation.
@martinpfannemueller gentle ping when you have some free time if is possible to add some JavaDoc as @mtrezza pleases, so this can be merged. Looking forward to see this merged 🤞 |
I just added some JavaDoc to allowCustomObjectId() in the configuration builder and the isAllowCustomObjectId() getter. |
There also seems to be a lint fix needed, see the failing lint check |
I updated the JavaDoc and ran spotlessApply |
Thanks, let just see whether the CI passes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for this PR!
# [2.1.0](2.0.6...2.1.0) (2021-11-21) ### Features * add support for custom objectId ([#1088](#1088)) ([d420371](d420371))
🎉 This change has been released in version 2.1.0 |
I implemented the custom objectId feature mentioned in #1087 including unit tests. Is there something missing to get this merged?
Thanks!