-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Rework release preparation tasks #44
Conversation
87facb4
to
9ae63d1
Compare
Puppet 6 bundle Ruby 2.5, do you think it's time to remove Ruby 2.4 from CI? |
9ae63d1
to
2ab5672
Compare
Do we also need to update the readme for these changes? |
I think by now that's fine. All our other tooling has dropped 2.4. |
only our puppet-lint plugins test on 2.4. mostly because I didn't want to do a major release. |
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.
To share some related work: create_gch_task already exists. It may not be suitable for us now, but it can serve as an inspiration.
Ruby 2.4 is EoL. We drop it because newer features won't work with Ruby 2.4 anymore: voxpupuli#44
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.
Looks good to me. Did you test it in any module? I moved the Ruby 2.4 drop to another PR, for a cleaner changelog: #46
lib/voxpupuli/release/rake_tasks.rb
Outdated
options = GitHubChangelogGenerator::Parser.default_options | ||
options[:user] = GCGConfig.user | ||
options[:project] = GCGConfig.metadata['name'] | ||
options[:future_release] = "v#{GCGConfig.metadata['version']}" if GCGConfig.metadata['version'] =~ /^\d+\.\d+.\d+$/ |
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.
Pretty sure not all modules use a v
prefix. Ideally it's configurable in GCGConfig
.
Also, if you don't use the match object then in modern Ruby it's preferred to use .match?
.
options[:future_release] = "v#{GCGConfig.metadata['version']}" if GCGConfig.metadata['version'] =~ /^\d+\.\d+.\d+$/ | |
options[:future_release] = "v#{GCGConfig.metadata['version']}" if GCGConfig.metadata['version'].match?(/^\d+\.\d+.\d+$/) |
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.
Pretty sure not all modules use a
v
prefix. Ideally it's configurable inGCGConfig
.
I used to exclude the v
because I like tag to be the semver version, but this is unfortunately somewhat broken with GCG and I gave up. As far as I can see, we use a v
in the modules tags, and the issues is only when GCG create the links to pages references containing tag that do not exist: if you mix v1.2.3
and 1.2.3
in your existing tags, GCG generates the good url. So not sure it's worth it to have a way to tune something that does not seem broken.
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.
out of 170 modules managed by modulesync on my machine at $WORK, only these do not have a 'v' at the beginning of their last tag:
- puppet-augeasproviders_mounttab
- puppet-augeasproviders_nagios
- puppet-augeasproviders_puppet
- puppet-augeasproviders_sysctl
- puppet-module
- puppet-puppet_summary
These module have not seen a new version recently, maybe we can "standardize" on version numbers with a leading 'v'?
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.
In Foreman we don't have it, and I'd love to standardize the ecosystem in some way, so I'd prefer to the conditional part.
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.
Thinking more about it: tools often have a "tag pattern". That could default to v%s
and some modules could override it.
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 makes sense 👍
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.
Did this in a new commit (with the other part in voxpupuli/modulesync_config#789). While doing this I realized that the pattern did not match the version we produce after creating a release ("<released-version.minor+1>-rc0", so I though we could remove it.
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.
The idea behind NOT matching rc0 is that we end up generating no future version. That means it ends up being "Unreleased" in the generated changelog. Though I'm not sure if this actually matches the real world.
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.
Got it! 👍 I amended my last commit accordingly
8b8d658
to
ce5bc70
Compare
I run the |
@ekohl I know you're quite busy at the moment. Could you please take a look in the next few weeks when you've some spare time? If you like we could also go on with it and implement possible enhancements in the future. |
bundle exec rake release:prepare | ||
``` | ||
|
||
Commit these changes (likely in a new branch, open a Pull-Request and wait for it to be reviewed and merged). When ready to ship the new release, ensure you are on the main branch, it is up-to-date, and run: |
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.
Is it possible for the task to create a new branch and create the commit with a standardised commit message? I'd like to see Release x.x.x
used consistently.
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.
/me feels like it is too much 😄
Would we can just adjust the message and suggest things:
Please review these changes and commit them to a new branch:
git checkout -b release-x.x.x
git commit -m "Release x.x.x"
Then open a Pull-Request and wait for it to be reviewed and merged).
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 had this as local changes in my working directory, so I added a commit with it while rebasing.
This is a follow-up to voxpupuli/modulesync_config#788 Voxpupuli contributors used to update a module version and `rake changelog` before opening a PR for preparing a new release. Some modules now have a REFERENCE.md file that also needs to be updated (by running `rake reference`), and forgetting to do so will remain unnoticed until `rake release` complain that the file is not up-to-date (so after the release PR was reviewed and merged). This PR move the `changelog` and `reference` tasks from each module's Rakefile (under modulesync control), to the voxpupuli-release gem, rename them to `release:porcelain:changelog` and `release:porcelain:reference` to leave room for new `changelog` and `reference` tasks indicating they are new deprecated, and recommand running the new `release:prepare` task that runs them. This is indented to help maintainers smoothly update their workflow and provide a framework where additional tasks can be added if need be.
Include information about the release:prepare task and detail what happen at each stage of the release process.
Co-authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
Tell users to run `strings:generate:reference` directly if needed.
Make this dependency optional. If absent, add a dummy task that describes how to make it functional.
Stop using the GitHubChangelogGenerator rake task generator in order to lazy-evaluate our new GCGConfig.user which defaults to whatever is declared in the module metadata but can be overriden in the module Rakefile.
ce5bc70
to
7f88849
Compare
@alexjfisher, @ekohl I think this is ready for merging, but I would love some feedback on the open discussions if you have some time. Thanks! |
7f88849
to
4d5ee4f
Compare
4d5ee4f
to
763950d
Compare
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.
Sorry for the string of reviews. I tend to find more things as time passes. I think this is now pretty close to being merged and we can solve these later if you want.
lib/voxpupuli/release/rake_tasks.rb
Outdated
|
||
desc "Prepare a release" | ||
task "release:prepare" do | ||
v = Blacksmith::Modulefile.new.version |
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 looks unused
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 is only used for end-user convenience to output the CLI commands to create the release branch:
git checkout -b release-#{v}
git commit -m "Release #{v}"
lib/voxpupuli/release/rake_tasks.rb
Outdated
rescue LoadError | ||
desc "Dummy" | ||
task "release:porcelain:changelog" do | ||
puts "Skipping CHANGELOG.md generation. Ensure github_changelog_generator is present if you expected it to be generated." |
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.
Shouldn't this hard fail instead?
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 we wanted to avoid a hard-failure for projects using our tooling but not hosted on GitHub and where GCG is not usable. I have never tested this however and maybe it is not functional at all… For now I would prefer to keep that this way if you don't mind.
The same way we can customize the user, allow customization of the project name.
ccc4d79
to
9bdfcb7
Compare
Code was moved around and is more complex that necessary. Make it more simple.
Follow rake best-practices.
Yeah, lots of moving parts! I addressed the last issues you raised and marked them as resolved, for the 2 last one I think we can left the code as it is for now. Thank you for your reviews! |
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.
The GCGConfig part is blocking, but otherwise 👍
This is starting to really shape up to be a big improvement over our current implementation. I think it's pretty much the last bit that I want to kill in what we modulesync, so thanks for the efforts!
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.
Thanks!
In this branch we include what is required to validate this two PRs: * voxpupuli/modulesync_config#789 (Synced from this branch) * voxpupuli/voxpupuli-release#44 (Fetch the gem form the WIP branch in git rather than Rubygems)
In this branch we include what is required to validate this two PRs: * voxpupuli/modulesync_config#789 (Synced from this branch) * voxpupuli/voxpupuli-release#44 (Fetch the gem form the WIP branch in git rather than Rubygems)
In this branch we include what is required to validate this two PRs: * voxpupuli/modulesync_config#789 (Synced from this branch) * voxpupuli/voxpupuli-release#44 (Fetch the gem form the WIP branch in git rather than Rubygems)
In this branch we include what is required to validate this two PRs: * voxpupuli/modulesync_config#789 (Synced from this branch) * voxpupuli/voxpupuli-release#44 (Fetch the gem form the WIP branch in git rather than Rubygems)
This is a follow-up to voxpupuli/modulesync_config#788
Voxpupuli contributors used to update a module version and
rake changelog
before opening a PR for preparing a new release. Some modules now have a REFERENCE.md file that also needs to be updated (by runningrake reference
), and forgetting to do so will remain unnoticed untilrake release
complain that the file is not up-to-date (so after the release PR was reviewed and merged).This PR move the
changelog
andreference
tasks from each module's Rakefile (under modulesync control), to the voxpupuli-release gem, rename them torelease:porcelain:changelog
andrelease:porcelain:reference
to leave room for newchangelog
andreference
tasks indicating they are new deprecated, and recommand running the newrelease:prepare
task that runs them.This is indented to help maintainers smoothly update their workflow and provide a framework where additional tasks can be added if need be.