Skip to content

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Aug 9, 2013

See this for the skeleton format I went with: https://github.com/jrafanie/linux_admin/blob/add_changelog/CHANGELOG.md

From the second commit:

The normal workflow should be:
1) Make your linux_admin code changes.
2) Test and commit these changes.
3) When ready for a new tag...
4) Run rake release
5) This will invoke the update_changelog rake task, it will ask you to:
6) Update LinuxAdmin::VERSION.
7) Verify that the script correctly added the commits since the last tag to the beginning of the CHANGELOG.md.
8) Add the correct version information before these commits.
9) Commit these changes
10) Hit enter and rake release will be run to create the tag, and push the version to rubygems.org.

The normal workflow should be:
1) Make your linux_admin code changes.
2) Test and commit these changes.
3) When ready for a new tag...
4) Run rake release
5) This will invoke the update_changelog rake task, it will ask you to:
6) Update LinuxAdmin::VERSION.
7) Verify that the script correctly added the commits since the last tag to the beginning of the CHANGELOG.md.
8) Add the correct version information before these commits.
9) Commit these changes
10) Hit enter and rake release will be run to create the tag, and push the version to rubygems.org.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One note, if you screw up, rake release will complain if you forgot to commit your changes locally.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 93d5de5 on jrafanie:add_changelog into 214a622 on ManageIQ:master.

@jrafanie
Copy link
Member Author

jrafanie commented Aug 9, 2013

@Fryguy @brandondunne @movitto - please review.

@Fryguy
Copy link
Member

Fryguy commented Aug 9, 2013

This is cool. Can we generalize this in some way for all of the other gems we have? Perhaps update the documentation in the CFME wiki to include this for all new gems we create?

The only (minor) thing I don't like about this is that it relies completely on commit messages. Those typically aren't always formatted nicely, and sometimes contain pointless things like "Fixed typo". Perhaps a followup step could be to clean up the generated changelog into reasonable features or and groupings (enhancements/bug fixes/removals - +/!/-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa, when did they add File.write? This changes everything.

Also, you should probably use interpolation here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, File.read and File.write are nice shortcuts.

Yeah, if this proof of concept is something we want to do, I'll clean it up.

@bdunne
Copy link
Member

bdunne commented Aug 9, 2013

@Fryguy good idea with editing the file before commit.

Maybe the tool should write the file then launch the git editor (git config --get core.editor) so that the user has a chance to edit. Once editor is closed, the script can commit the change.

@jrafanie
Copy link
Member Author

jrafanie commented Aug 9, 2013

@Fryguy Yeah, bundler's CHANGELOG.md splits it up into features vs. bugs. We can certainly do this but it's hard to tell one vs. the other. As it is now, the script just adds the commits to the changelog and you have to add the version that those commits refer to so at that point, it would make sense to review what should be cleaned up.

@brandondunne suggested rake release_patch/release_minor/release_major for automatically updating the version file, depending if we did that, we'd have change this around somewhat.

@bdunne
Copy link
Member

bdunne commented Aug 9, 2013

Another idea:
"rake release(:minor)" or if you run "rake release" it can prompt for major, minor or build then bump the version file automatically.

@jrafanie
Copy link
Member Author

jrafanie commented Aug 9, 2013

@Fryguy Yeah, if we follow semantic versioning and keep clean commit messages, it should be simple to use in other gems.

@bdunne
Copy link
Member

bdunne commented Aug 9, 2013

@jrafanie maybe once we have this working the way we want it we can push it upstream?

@jrafanie
Copy link
Member Author

jrafanie commented Aug 9, 2013

@brandondunne @Fryguy
Let's put our suggestions in this pull and then we can reach consensus on monday, push, and and release a new tag ;-)

👎 rake release:minor - I think it's a bad idea to hijack the release task and create a namespace of the same name.

👍 rake release - "enhance" release task as I currently am doing and ask whether it's a major/minor/patch version bump and automatically update the version file and put the correct tag name in the changelog

👍 open the "git editor" (git config --get core.editor), after close, auto stage and commit version and changelog. Although it's a little concerning if someone (me) closes the file in their editor with typos/mistakes, they don't have chance to review it again before the tag would be created.

@Fryguy
Copy link
Member

Fryguy commented Aug 9, 2013

Based on your concerns about accidentally tagging a bad commit from changelog issues, perhaps rake update_changelog should be run before a release and submitted as a proper PR for review? Then once it's merged in, we run rake release.

I'm concerned about slamming the two together...I realize it makes for better automation overall, but I'm concerned that when it doesn't work it will cause more headache than necessary.

@bdunne
Copy link
Member

bdunne commented Aug 9, 2013

@jrafanie Do you think you will do much editing of the changelog? I think it should force us to write better commit messages and keep as much automation as possible.

They are two different things. Maybe they should be split into two independent prereq's?

@jrafanie
Copy link
Member Author

jrafanie commented Aug 9, 2013

@brandondunne Yeah, if we commit correctly, it shouldn't be a big issue and I would be less concerned about pushing a bad tag.

If we keep the changelog as straight commits as it is now, then it would just be an occasional manual change to the changelog (for example to clean up a horrible commit message).

@Fryguy's idea of splitting/squashing commit messages in the changelog and segregating based on bug vs. feature is something completely else and makes automation much harder.

@bdunne
Copy link
Member

bdunne commented Aug 9, 2013

@jrafanie true.
We could enhance @Fryguy's idea and only add to the change log lines that begin with "bug:" or "feature:" That would cut down on the number of lines in the changelog (no more "fix typo") and ensure that they are categorized.

@movitto
Copy link
Contributor

movitto commented Aug 12, 2013

Reading back, just a couple brief comments:

  • Whats the release cycle of this gem? Based on current commit volume, if its not less than a couple months it might just be worth manually updating the changelog / version every so often
  • I've seen standardized commit messages in practice, but not terribly often. IMO it adds a bit of overhead to the process / makes it a bit more difficult to rope in new contributors. In general I'm ok with it if the benefit outweights the overhead.
  • I'd recommend against having a git push in the task that updates the CHANGELOG and VERSION files. Everything up to a staged commit would be fine so long as the user has a chance to review the proposed changes before pushing to the remote repo.
  • Along w/ this, git commit allows you to abort the commit if you exit the editor without saving your changes to your commit message. Would be nifty if we could do this when editing the changelog if possible

Overall would be cool if it ends up cutting back on the overhead.

@bdunne bdunne mentioned this pull request Sep 7, 2013
@cfme-bot
Copy link
Member

Checked commits jrafanie@6701a6f .. jrafanie@93d5de5 with rubocop
1 file checked, 1 offense detected

Rakefile

@jrafanie jrafanie closed this Jul 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants