Skip to content
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

docs: add installation migration info #427

Merged
merged 4 commits into from
Oct 21, 2022
Merged

Conversation

mtrezza
Copy link
Member

@mtrezza mtrezza commented Oct 20, 2022

New Pull Request Checklist

Issue Description

No instructions for installation migration.

Related issue: #426

Approach

Add instructions for installation migration.

TODOs before merging

n/a

@parse-github-assistant
Copy link

Thanks for opening this pull request!

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

@mtrezza mtrezza requested a review from a team October 20, 2022 19:58
@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Base: 90.33% // Head: 90.34% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (e28506d) compared to base (6956072).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
+ Coverage   90.33%   90.34%   +0.01%     
==========================================
  Files         161      161              
  Lines       16267    16267              
==========================================
+ Hits        14694    14696       +2     
+ Misses       1573     1571       -2     
Impacted Files Coverage Δ
Sources/ParseSwift/Objects/ParseUser+async.swift 93.11% <0.00%> (-0.41%) ⬇️
Sources/ParseSwift/Objects/ParseObject.swift 90.12% <0.00%> (-0.23%) ⬇️
Sources/ParseSwift/Objects/ParseObject+async.swift 96.59% <0.00%> (+0.37%) ⬆️
Sources/ParseSwift/Coding/ParseEncoder.swift 78.14% <0.00%> (+0.45%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.


After migrating an app to the Parse Swift SDK, launching the app will create a new `_Installation` object with a new `installationId`. It will appear as if the app had been uninstalled and then reinstalled, even though it was only updated with the new Parse Swift SDK.

This may be problematic if the installation object is directly referenced in your app or if it contains fields that should be maintained, like the `badge` number field for example. To address this, here are two options:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t understand what message this line is attempting to convey

Copy link
Member Author

@mtrezza mtrezza Oct 21, 2022

Choose a reason for hiding this comment

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

If the app references the current installation object for any purpose, that reference will be incorrect, because that's not the current installation anymore, even though the user didn't reinstall.

Same issue with data like the badge counter, which will suddenly become zero for example, which has an effect on the user as well. That data may need to be migrated to the new installation.

Any suggestions to make that more clear?

MIGRATION.md Show resolved Hide resolved
@cbaker6
Copy link
Contributor

cbaker6 commented Oct 21, 2022

You can handle adding and reviewing any future changes to this document since you tend to ignore any changes you don’t like or even worse, you chose to commit to the main branch whenever you want. In the other repos, you force others to conform to your reviews and changes and hold up the PR’s, but don’t hold yourself to the same standard. Others have complained about this as well. I have no other comments on this document or discussion related to migration, so don’t reference me when questions come up as it’s a waste of my time.

@mtrezza
Copy link
Member Author

mtrezza commented Oct 21, 2022

If you have any specific suggestions I'm open to discuss, but you haven't put anything forward. Instead you are opening fundamental discussions to which standard we should hold this SDK. The reality is that developers are switching over from the ObjC SDK and are asking how to do that. The Swift SDK needs to address that, its not a walled-off standalone product. It's part of Parse Platform and it's our obligation to be mindful about the needs of developers and provide a central document for switching from ObjC to Swift SDK. I'm repeating my invitation to discuss your viewpoints in chat and I will reach out to you on another channel.

On another note, I came across comments where you advertise this SDK as having "more features than the Objective-C SDK". This is incorrect, as we see with offline storage. Please don't do that. It can lead to unpleasant surprises for developers. That's the reason why we're compiling this migration guide, to give a comprehensive overview about the differences.

If you don't want to be tagged for PR reviews in this repo, I can only remove you from the review team. We don't tag specific individuals to review PRs, unless they are not on that team but they are related to a specific PR.

@mtrezza mtrezza merged commit 8aefc95 into main Oct 21, 2022
@mtrezza mtrezza deleted the docs-migration-installation branch October 21, 2022 12:56
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.

2 participants