-
Notifications
You must be signed in to change notification settings - Fork 285
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
#1515
Deprecate Deliverfile
in favor of Fastfile
#1515
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.
Similar to woocommerce/woocommerce-ios#8831 (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 (see wordpress-mobile/WordPress-iOS@6b88866)
I took a look and indeed it seems that getting rid of the old lane makes sense as the new lane also takes care of what the old lane covered. And as it happens, I checked in our release scenario for Simplenote-iOS (private repo) and couldn't find a reference to that old
Good point and nice to have noticed that 👍 . But that's all right. Simplenote is currently in maintenance mode, so that's not surprising that it's on an older version of the toolkit, and we don't plan to update it that soon (we don't know when it's going to go out of maintenance mode and we'll have time to work back on it), so we kinda already know that there will be more work to be done on other places in its |
Ah, it makes sense then for it to be removed and the functionality done in that single lane. Thanks for checking! 👍
Got it 👍 Yeah, indeed it will need more work to catch up with the latest changes. |
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 theFastfile
. For that I also moved out theprivacy_url
and thecopyright
to separate files in themetadata
folder.I assumed the existing lane
upload_screenshots
could then be removed in favour of the new lane,update_metadata_on_app_store_connect
, which will be responsible for both the metadata upload as well as the screenshots upload.Additionally, the need for an
app_version
set in theDeliverfile
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):
Also good to test the use of the parameter
with_screenshots:true
to see that it will attempt to load the screenshots.Review
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.
Release
These changes do not require release notes.