Skip to content

feat: add ParseObjectMutable protocol #270

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

Merged
merged 6 commits into from
Oct 31, 2021
Merged

feat: add ParseObjectMutable protocol #270

merged 6 commits into from
Oct 31, 2021

Conversation

vdkdamian
Copy link
Contributor

@vdkdamian vdkdamian commented Oct 30, 2021

New Pull Request Checklist

Issue Description

Related issue: #269

Approach

Make emptyObject more developer friendly.

TODOs before merging

  • Add tests
  • Add entry to changelog
  • Test the code
  • Add changes to documentation (guides, repository pages, in-code descriptions)

Notes

Before I put more work into this, @cbaker6 - @dblythy what's your opinion about this approach?

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title feat: Updatable feat: updatable Oct 30, 2021
@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 30, 2021

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@codecov
Copy link

codecov bot commented Oct 30, 2021

Codecov Report

Merging #270 (6b21e28) into main (126f4f6) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #270      +/-   ##
==========================================
- Coverage   82.34%   82.33%   -0.01%     
==========================================
  Files          99      100       +1     
  Lines       10512    10518       +6     
==========================================
+ Hits         8656     8660       +4     
- Misses       1856     1858       +2     
Impacted Files Coverage Δ
Sources/ParseSwift/Objects/ParseObject.swift 74.57% <ø> (ø)
Sources/ParseSwift/Types/ParseError.swift 100.00% <ø> (ø)
...rces/ParseSwift/Protocols/ParseObjectMutable.swift 100.00% <100.00%> (ø)
Sources/ParseSwift/LiveQuery/ParseLiveQuery.swift 73.08% <0.00%> (-0.30%) ⬇️

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 126f4f6...6b21e28. Read the comment docs.

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 30, 2021

In order to properly make sure this runs through the CI, you will need to replace empty object in the test suite here

with the protocol you added. In that file, you will need to switch emptyObject to the new property name in the protocol for the emptyObject tests.

Since this protocol is optional and something the developer choses to add themselves. There wouldn't be much else that needs to be added to the SDK besides cleaning up the documentation.

With this said, I think you can add all of this now which will allow us to asses the feasibility and simplicity (the test suite). In theory it should work as the emptyObject property was part of the ParseObject protocol in a previous version. This PR moves the emptyObject into an optional protocol the developer can choose to conform to or not.

@cbaker6 cbaker6 linked an issue Oct 30, 2021 that may be closed by this pull request
3 tasks
@cbaker6 cbaker6 marked this pull request as draft October 30, 2021 11:37
@vdkdamian vdkdamian changed the title feat: updatable feat: ParseObjectMutable Oct 30, 2021
@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title feat: ParseObjectMutable feat: parseObjectMutable Oct 30, 2021
@vdkdamian vdkdamian closed this Oct 30, 2021
@vdkdamian vdkdamian deleted the updatable branch October 30, 2021 22:32
@vdkdamian vdkdamian restored the updatable branch October 30, 2021 22:33
@vdkdamian vdkdamian reopened this Oct 30, 2021
@vdkdamian vdkdamian requested a review from cbaker6 October 30, 2021 22:35
@cbaker6 cbaker6 changed the title feat: parseObjectMutable feat: add ParseObjectMutable protocol Oct 31, 2021
@cbaker6
Copy link
Contributor

cbaker6 commented Oct 31, 2021

You will need to pull before you add the requested changes as I resolved conflicts in this branch

@vdkdamian vdkdamian requested a review from cbaker6 October 31, 2021 09:18
@vdkdamian vdkdamian requested a review from cbaker6 October 31, 2021 13:29
@cbaker6
Copy link
Contributor

cbaker6 commented Oct 31, 2021

It's looking good, just a for more nits!

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 31, 2021

click the Ready for review button when you finish the updates

@vdkdamian vdkdamian marked this pull request as ready for review October 31, 2021 20:46
@vdkdamian vdkdamian requested a review from cbaker6 October 31, 2021 22:21
@cbaker6
Copy link
Contributor

cbaker6 commented Oct 31, 2021

LGTM!

@cbaker6 cbaker6 merged commit 321905d into parse-community:main Oct 31, 2021
@vdkdamian vdkdamian deleted the updatable branch November 1, 2021 21:32
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.

Make emptyObject more developer friendly
2 participants