-
Notifications
You must be signed in to change notification settings - Fork 97
Check if maven has published current version before deploying #2799
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
base: main
Are you sure you want to change the base?
Check if maven has published current version before deploying #2799
Conversation
| version = options[:version] || current_version_number | ||
| UI.message("Checking if version #{version} already exists in Maven Central...") | ||
|
|
||
| group_id = "com.revenuecat.purchases" |
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.
I'm assuming the group id, which should be fine for this repo.
| includes.from("README.md") | ||
| } | ||
|
|
||
| tasks.register("listPublications") { |
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.
Honestly not sure if it's worth the extra complexity here. The idea is to be able to know what we publish to maven dynamically so, if we add more modules we publish in the future, we don't need to update any hardcoded list.
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.
I think this is nice!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2799 +/- ##
=======================================
Coverage 77.89% 77.89%
=======================================
Files 315 315
Lines 12161 12161
Branches 1686 1686
=======================================
Hits 9473 9473
Misses 1965 1965
Partials 723 723 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JayShortway
left a comment
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.
Nice improvement! Just some more ideas that came to mind 😄
| includes.from("README.md") | ||
| } | ||
|
|
||
| tasks.register("listPublications") { |
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.
I think this is nice!
| end | ||
|
|
||
| desc "Check if version already exists in Maven Central" | ||
| lane :validate_version_not_in_maven_central do |options| |
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.
Are we going to move this to the Fastlane plugin? That way we can reuse for PHC and KMP.
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.
Indeed, I think it will be needed there as well. Will try to move it there instead
Edit: Waiting until it's ready to have all the reviews in one place. Once it's ready, I will move to the plugin.
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.
I extracted part of the logic here to the plugin: RevenueCat/fastlane-plugin-revenuecat_internal#101. We would still need to keep some logic to get the artifacts to check... but this felt more like a gradle task, and separate from the plugin action... I guess we could assume the gradle command would be available in all SDKs using it but preferred to keep it separate for now. lmk what you think there! (still need to adapt this PR to use that lane from the plugin)
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.
(This comment is not related to this file at all 😄)
Another idea I had was to separate the upload step from the publish step, (plugin docs), and only publish at the end, once we verify that all packages have been uploaded. (And if not, we could e.g. drop whatever we did upload to try again.)
We can publish (and drop) uploaded packages using the API.
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.
Hmm indeed I think it's another valid approach... But I guess there is still a chance of the process being stopped midway in this approach when publishing right? Not sure if having all the artifacts uploaded would be enough for the plugin to fail on the next attempt though...
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.
there is still a chance of the process being stopped midway in this approach when publishing
Hmm good point, but this should be much faster (seconds vs minutes), so the chance is very low I'd say.
Not sure if having all the artifacts uploaded would be enough for the plugin to fail on the next attempt though...
If we (are able to) check uploaded artifacts, we could make the release job fail if it finds any.
|
|
||
| auth_token = ENV["FETCH_PUBLICATIONS_USER_TOKEN_MAVEN_CENTRAL"] | ||
| if auth_token.nil? || auth_token.empty? | ||
| UI.user_error!("FETCH_PUBLICATIONS_USER_TOKEN_MAVEN_CENTRAL environment variable is not set. Please provide a valid token to check Maven Central publications.") |
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.
This has already been set in CircleCI android (need to add it in PHC and KMP once we move this to the plugin)
vegaro
left a comment
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.
I think this makes sense. It won't prevent race conditions (imagine another job in parallel actually publishing) but I don't think we can do much about it
Description
This adds some checks to the deploy job to make sure we don't start deploying if the version trying to be deployed has already been fully or partially deployed.