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

Deprecate Deliverfile in favor of Fastfile #1084

Merged
merged 2 commits into from
Feb 10, 2023

Conversation

iangmaia
Copy link
Contributor

@iangmaia iangmaia commented Feb 6, 2023

Fix

Similarly to what was done for WPiOS on wordpress-mobile/WordPress-iOS#16805, this PR removes the Deliverfile moving everything to a new lane in the Fastfile. For that I also moved out the privacy_url and the copyright to separate files in the metadata folder.

Additionally, the need for an app_version set in the Deliverfile has been deprecated and its usage is gonna soon be removed altogether from release-toolkit, so this PR already cleans this up.

Test

Testing the new lane is similar to wordpress-mobile/WordPress-iOS#16805 (and is particularly difficult for me to do completely, so I'd also appreciate some help):

  1. Checkout this branch
  2. Run bundle exec fastlane update_metadata_on_app_store_connect
  3. Verify the Preview.html contains valid data. In particular, that it's not attempting to upload screenshots
  4. Cancel the upload 🙏 Does the Preview on path './fastlane/Preview.html' look okay for you? (y/n) n

Also good to test the use of the parameter with_screenshots:true to see that it will attempt to load the screenshots.

Review

I have noticed that this project uses an older version of the release-toolkit, so perhaps this upgrade needs to happen before this PR lands.

Release

These changes do not require release notes.

Copy link
Contributor

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

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

Similar to woocommerce/woocommerce-ios#8831 (review) and Automattic/simplenote-ios#1515 (review), we need to add skip_deliver: true to the call site which will try to bump the version from the Deliverfile now that that file is gone

(this is something we also forgot to do the first time we did such a change in WPiOS… which required a follow-up commit to fix it later…)

@AliSoftware
Copy link
Contributor

AliSoftware commented Feb 10, 2023

Since one lane was removed and another one was added, I imagine the release process needs to be amended as well. Please review carefully the parameters used in the new lane update_metadata_on_app_store_connect, making sure it covers the current use cases.

I have also noticed that this project uses an older version of the release-toolkit, so perhaps this upgrade needs to happen before this PR lands.

Good points, and same answers as the ones I gave in Automattic/simplenote-ios#1515 (comment):

  • Current scenario for Simplenote Mac (private repo) doesn't mention the upload_screenshots lane you deleted, so there won't be a need to amend the release process to account for the change in this PR. (If anything, we could consider adding a missing step to the Simplenote release process to mention calling this new lane if necessary…)
  • Simplenote-macos is in maintenance mode right now, so no worries about it being on older release-toolkit (though that was nice of you to notice). We already know that we'll need more than just the work you've been doing with these PRs before we're ready to migrate to a newer release-toolkit (even just to the existing 6.x), but at least your PR makes us closer to that goal now 🙂

@iangmaia iangmaia marked this pull request as ready for review February 10, 2023 18:34
@iangmaia iangmaia requested a review from AliSoftware February 10, 2023 18:35
@AliSoftware AliSoftware merged commit 6853f5e into Automattic:trunk Feb 10, 2023
@iangmaia iangmaia deleted the cleanup/deprecate-deliver-file branch February 24, 2023 12:38
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