Fix crashes after testing beta lanes post 1.3.0#283
Conversation
…HA_VERSION env var This is a temporary workaround, and defaults to zalpha if HAS_ALPHA_VERSION is not provided or is =1 The long term solution would be to provide the alpha flavor name as part of the parameters/ConfigItem of each action needing it. (But we need a quick fix for now)
AliSoftware
force-pushed
the
jetpack/fix-crashes
branch
from
782ece0 to
262e680
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #283 +/- ##
===========================================
+ Coverage 51.62% 51.66% +0.04%
===========================================
Files 108 108
Lines 4527 4531 +4
===========================================
+ Hits 2337 2341 +4
Misses 2190 2190 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| release_version = Fastlane::Helper::Android::VersionHelper.get_release_version(product_name: product_name) | ||
| message << "#{app}] Looking at branch release/#{version} as requested by user. Detected version: #{release_version[Fastlane::Helper::Android::VersionHelper::VERSION_NAME]}.\n" | ||
| alpha_release_version = Fastlane::Helper::Android::VersionHelper.get_alpha_version | ||
| alpha_release_version = Fastlane::Helper::Android::VersionHelper.get_alpha_version(product_name) |
There was a problem hiding this comment.
Side note: we need to harmonize the parameter names for all get_*_version helper methods at some point, so that we're consistent with using product_name: named parameter for all of those, instead of having a mix like here.
But that's something for another day, as the goal of this PR is just to make things work and stop crashing, limiting any refactoring to a minimum for now; and fixing consistency and doing the clean up only later.
|
|
||
| * Support for a `version.properties` to manage app versioning - all existing paths remain intact and new paths are only used when a `version.properties` file is present. | ||
| * Add support for providing an `app:` parameter to most versioning-related actions to allow support for multiple apps hosted in a monorepo | ||
| * Add support for providing an `app:` parameter to most versioning-related actions to allow support for multiple apps hosted in a monorepo. |
There was a problem hiding this comment.
I'm still not convinced we need / should maintain this given the fact that we're using a common version between apps sharing a repo right now. I feel like this will be a future maintenance burden for something we might need in the future. WDYT?
There was a problem hiding this comment.
@jkmassel I agree, but this change was already shipped in 1.3.0 (this entry is from ## 1.3.0, the only change I made on this line is to fix a missing .)
My take is that we should at least avoid having the toolkit crash in its current 1.3.0 version, so we should still land this 1.3.1 hotfix as is, to at least get back to a non-crashing version.
And only after that lands, revert the changes as we discussed (to get back to using a common version for both WP and JP, and remove that app: parameter) for a future 1.4.0 – well actually a 2.0.0 as this would be a breaking change I think. It would have maybe be better to not land 1.3.0 at all, but here we are, so I think that's the best course of action.
Does this make sense? If so, can you approve the PR so it can be merged in the meantime before I work on reverting those changes later?
3 participants
As I started using the new
1.3.0release-toolkit to generate WordPress 17.6-rc-4, Jetpack 17.6-rc-4, I encountered some crashes, that this PR intends to fix.Depending if I run into issues during the final release on Friday, I might need to ad more fixes to this PR before we consider a new release, but the changes that are in so far are already improving the current state and ready to be reviewed.
\cc @JavonDavis for your information