Skip to content
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

A way to get rid of fromLine #95

Open
lurch opened this issue Oct 14, 2017 · 8 comments
Open

A way to get rid of fromLine #95

lurch opened this issue Oct 14, 2017 · 8 comments

Comments

@lurch
Copy link
Contributor

lurch commented Oct 14, 2017

There was recently a slight slip-up in the mountutils repo, where the CHANGELOG.md wasn't in the correct format balena-io-modules/mountutils#50 (which ironically #90 would have prevented).
There were fewer than expected lines in the "changelog header" which meant that the default fromLine: 6 value also skipped over the leading version line (in this case ## v1.2.2 - 2017-08-28 which caused the version-number parsing of versionist to get confused, which caused "unexpected results" - VersionBot bumped the version to 1.3.0 when we were expecting it to bump to 1.2.3.

Once we'd figured out what the problem was, @jhermsmeier suggested that one way to avoid this in future might be to just look for the first version-line, rather than always skipping a hardcoded number of lines.
And I guess to do that, we'd need to be able to convert the first line of the template into a regex, which for the default template would mean converting ## v{{version}} - {{moment date "Y-MM-DD"}} into the regex /## v\d+\.\d+\.\d+ \- \d{4}\-\d{2}\-\d{2}/ (which I guess in turn would require versionist to "understand" the templating system being used). And it looks like my semver-regex is too simplistic ;-) https://github.com/sindresorhus/semver-regex/blob/master/index.js

@hedss
Copy link
Contributor

hedss commented Oct 16, 2017

So, for this to be usable we need versionist as a NPM module still so checks can be run before any file-altering changes... any takers? #83

@craigmulligan
Copy link
Contributor

I'd be interested @hedss, but I'm super busy this week and on leave till the end of the month. Perhaps you can spec out what the api would look like and I can implement when I'm back?

@hedss
Copy link
Contributor

hedss commented Oct 16, 2017

@craig-mulligan I'll have a think and see if how the onprem stuff goes ;-) And thank you very much for the offer! :)

@lurch
Copy link
Contributor Author

lurch commented Dec 1, 2017

The identical thing also tripped up @pimterry recently in balena-io-modules/balena-procbots#348

However I had a bit of a think, and rather than trying to build a template -> version-regex parser, I had an idea for an alternative (possibly easier?) way that we could get rid of fromLine and stop this error biting other people in future...

The problem that @jhermsmeier and @pimterry stumbled over is that the fromLine value in the default versionist.conf.js assumes that the CHANGELOG.md in their respective repos matches up with what https://github.com/resin-io/process/blob/master/process/Commit_and_PR_Guidelines.md#versionbot-automatic-versioning-review-requirements-and-changelog-generation mandates (but as there was a mis-match, everything went wonky).
So instead of having these values 'decoupled', we could instead get rid of

fromLine: 6

in versionist.conf.js and replace it with:

changelogHeader: [
  '# Change Log',
  '',
  'All notable changes to this project will be documented in this file',
  'automatically by Versionist. DO NOT EDIT THIS FILE MANUALLY!',
  'This project adheres to [Semantic Versioning](http://semver.org/).',
  ''
].join('\n')

and then when versionist runs it would check that the CHANGELOG.md indeed starts with these standard header lines, and throw a suitable error if not.
This would also have the additional benefit that all VersionBot-using projects would definitely have the boilerplate header that https://github.com/resin-io/process/blob/master/process/Commit_and_PR_Guidelines.md#versionbot-automatic-versioning-review-requirements-and-changelog-generation requires, rather than just ensuring that they have 6 random lines of text above the start of the changelog history ;-)

(Also pinging @jviotti , as the original Versionist author)

@pimterry
Copy link

^ This sounds very sensible to me, and would be a great improvement.

Even better, I personally wouldn't object to a special case that matched the exact standard header we used to use:

# Change Log

All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

so that if you have exactly that, it automatically replaces it with the new versionist-expected one, and then continues as normal. Seems like a reasonable special case for resin at least (perhaps not externally, but I think this is fairly common more widely too) and that way it'd be far more likely to work straight out of the box, and we'd skip an annoying little manual step we currently have to do over and over again (and which we keep forgetting).

@lurch
Copy link
Contributor Author

lurch commented Dec 11, 2017

Almost sounds like you're suggesting versionist should be able to run an optional search'n'replace pre-processing step over the CHANGELOG.md before then running the rest of the usual versionist steps? 😉
(and if such a feature was added, then maybe it'd make sense to also have an optional search'n'replace post-processing step, for symmetry?)

Although TBH I'm not sure if such special-case code is worth adding, as it only needs to be done once per repo, rather than every time versionist runs, and...

an annoying little manual step we currently have to do over and over again

Presumably at some point we'll have eventually made all our repos versionist-compatible? :)

(and which we keep forgetting)

If my above suggestion were implemented, then we wouldn't be allowed to forget because versionist would keep moaning at us until we fixed it ;-)

@pimterry
Copy link

If my above suggestion were implemented, then we wouldn't be allowed to forget because versionist would keep moaning at us until we fixed it ;-)

True, and that should definitely be the main fix! But we've still got a lot of repos to get through to migrate I think, and dropping all that work would be nice, if it's easy (but it sounds like it's not)

Really, what I'd like is a kind of codemod-for-repos, where I can write a transformation ("if there's a changelog that starts with X, replace it with Y"), and make the change and open a corresponding PR for all repos in an organisation automatically (and get a list of repos that didn't match to check manually too). That'd nicely solve this versionbot setup for most cases, but also handle all the current work to replace travis with circleci, and all sorts in future...

@lurch
Copy link
Contributor Author

lurch commented Dec 11, 2017

Really, what I'd like is a kind of codemod-for-repos, where I can write a transformation ("if there's a changelog that starts with X, replace it with Y"), and make the change and open a corresponding PR for all repos in an organisation automatically

Yeah, that sounds like a much better (and ultimately more flexible) solution than having versionist doing a specific search'n'replace on the Changelog.
And it probably shouldn't be too hard to script something like that using the GitHub API? Just needs a bit of time, which always seems to be such a scarce resource!

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

No branches or pull requests

4 participants