-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add Github workflow to allow for automatic preview releases #22367
base: 5.x-dev
Are you sure you want to change the base?
Add Github workflow to allow for automatic preview releases #22367
Conversation
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Check if there are any changes to create a preview release for | ||
id: changes |
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.
Have you thought about using a separate job just to check if there are changes to release?
Would have the overhead of an additional checkout if the release should be made, but it may simplify ending the pipeline early if there is nothing to release. And could remove most output checks in favor of just having needs
at the job level.
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.
That's a good suggestion, we could extract the check into a separate job. Haven't done it that way because it was the first thing in the workflow when I started working on it and the tests and release jobs were moved from steps to jobs subsequently and at that point I haven't thought about also putting the check to a job. Probably not urgent as the check is fast and there are not many expensive steps after that to skip.
core/Version.php
Outdated
return ''; | ||
} elseif ($this->isStableVersion($version)) { | ||
// no suffix yet | ||
return $version . '-alpha.' . $dt; |
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 seems incorrect. If we e.g. have released Matomo 5.3.0
, the next preview release should not be 5.3.0-alpha.XXXXXX
. We would need to increase the patch (or minor) release number in that case.
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.
Hmmmm, you might be onto something here. I checked the docs on the preview release versioning but I didn't think that for x.y.0
we would need to bump the patch version.
If that's the case (can you confirm @sgiehl and @mneudert) then I can adjust the logic here to check for x.y.0
versions specifically.
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.
It's not only around x.y.0
. If we have released e.g. 5.3.1, the next would need to be 5.3.2-alpha.
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.
Fair point, will adjust.
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.
@sgiehl this has been adjusted, please re-review. Thanks!
release_preview_version: | ||
needs: [run_matomo_tests] | ||
runs-on: ubuntu-latest | ||
steps: |
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 now duplicates most of the release action code. Any reason for not reusing the release action we already have here?
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.
It extracts just the main flow from the tag
step and removes all extra logic.. but yeah, technically we might be able to reuse the release action.
Do we want to use the release action here or are we happy with the current state @matomo-org/core-team?
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.
Personally I would vote for reusing the action, even if that would mean to adjust the other action so it also also works for releasing a preview on the new preview branch.
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.
Will adjust it here to use the action so that we only have one action producing releases.
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.
@sgiehl reused the release action, please re-review. Thanks!
core/Version.php
Outdated
return $version . '-alpha.' . $dt; | ||
} else { | ||
// -b1, -rc1, -alpha | ||
return $version . '.' . $dt; |
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.
Before simply appending that to an unstable release (other than alpha), I guess we would need to check if that version has already been released.
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.
That wouldn't be a concern for this method though. This provides the next preview based on what's in core/Version.php, in principle. Is there a possibility we will bump the version in core/Version.php and not release a version at the same time? I guess it can happen, but should it happen and would it be expected to happen at 1am NZ time? Happy to adjust if we deem it necessary, just wondering how likely it is to happen.
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.
But if we assume that the version stated in this file was already release, we would need to increase the beta / rc number as well, to ensure the returned version is higher.
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'll add a check to see if the version in the version file has been released already in case of stable and non-stable versions (beta, rc). If there's no such version this workflow will stop.
9eae8e3
to
7f8ac01
Compare
Co-authored-by: Marc Neudert <[email protected]>
bf8567c
to
f9dfc98
Compare
Description:
Adds a new workflow that determines what the new preview version should be, updates the version file, runs the tests for the new version and on success publishes a preview release.
If any of the steps fail along the way the action stops and doesn't create a release.
This introduces 5.x-preview branch that is always reset to the top of 5.x-dev before the version number is increased and tests are run, it can technically be deleted as it will be recreated every time.
The check for code changes is done between the newest preview version tag and 5.x-dev, excluding the line that contains the version number change in core/Version.php.
We can consider extending the range of changes we want to ignore in a follow up.
We either need to merge this for testing after a code review or people can run this in their own forks.
Review