Skip to content

Conversation

@cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Dec 22, 2020

This makes ParseGeoPoint consistent with the rest of the SDK.

File will be changed to ParseFile in a future PR.

@codecov
Copy link

codecov bot commented Dec 22, 2020

Codecov Report

Merging #39 (b0263de) into main (db391a4) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #39   +/-   ##
=======================================
  Coverage   72.87%   72.87%           
=======================================
  Files          30       30           
  Lines        2127     2127           
=======================================
  Hits         1550     1550           
  Misses        577      577           
Impacted Files Coverage Δ
Sources/ParseSwift/Parse Types/ParseGeoPoint.swift 98.27% <100.00%> (ø)
Sources/ParseSwift/Parse Types/Query.swift 87.68% <100.00%> (ø)

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 db391a4...b0263de. Read the comment docs.

@cbaker6
Copy link
Contributor Author

cbaker6 commented Dec 22, 2020

At one point, I wanted objects created in the SDK to be named GeoPoint (without "Parse" in front) and the protocols to ParseObject (with "Parse" in front), but this currently isn't possible because of ACL in ParseObject. The way the ParseObject is currently defined, we don't don't need to specify the CodingKeys directly since it conforms to the Codable protocol, which can encode/decode the property ACL directly. If we wanted to rename the property ACL to ParseACL (allowing us to what I originally wanted with GeoPoint), then we would need a custom decoder which would open up a can of worms that isn't worth messing with.

So now, all structs and protocols will have "Parse" in front, which seems to match the JS SDK.

@TomWFox what do you think?

@TomWFox
Copy link
Contributor

TomWFox commented Dec 24, 2020

I can't lie, I've read this a few times and I can't make sense of it 😅. I think having "Parse" in front is perfectly fine, and not worth opening up a can of worms.

Happy to go with your judgement.

@cbaker6
Copy link
Contributor Author

cbaker6 commented Dec 24, 2020

Lol, I was deep in the code when I wrote this, but this will make everything more consistent.

The only parse type that won't have "Parse" in front after this will be "Query", but a Query is always tied to a "ParseObject", so I don't think will be needed as it will be redundant. Developers won't be able to tie "Querys" to non-parse objects, so it will be okay

@cbaker6 cbaker6 merged commit 739db44 into main Dec 24, 2020
@cbaker6 cbaker6 deleted the geopoint branch December 24, 2020 22:30
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.

3 participants