product-os / versionist

Flexible CHANGELOG generation toolkit that adapts to your commit conventions
44 stars 15 forks source link

New user experience #68

Open nazrhom opened 7 years ago

nazrhom commented 7 years ago

I ran into the following issue while trying to configure versionist in one of my projects, I think it represents be a common enough scenario that may be worth handling explicitly.

Lets assume I am about to publish my module to npm. I have heard about semver and versionist and I want to adopt it in my project. After a quick glance at the README the most common course of action is:

npm i -g versionist

I then go to my project root and type

versionist

I was using a version prior to this PR #61 so I got an error here complaining about the lack of versionist.conf.js which triggered me to go and further read that part of the docs. I think #61 is a great idea but we could add a line saying No versionist.conf.js found. Using defaults, I think mentioning it explicitly is a good way to convey the message that one should really read and understand the configuration that is being used.

Once that is sorted out, you run versionist again and this time you get

Error: Cannot read property 'trim' of null

This has already been reported in #65 , however the solution presented there is still a bit confusing to new users imo. To get past the error in the scenario described above, I made an empty commit with body = "Initial Version" and a Change-Type: major footer. I was thinking this functionality (would it be possible to avoid having to create an empty commit?) might be a useful addition behind an appropriate flag like --init -m <message>. This would allow a new user to setup verionist quickly on his project, "squashing" every messy commit he made up to that point into a promise, to follow whatever configuration he picked, from now on. Moreover in case of an error like #65 we could detect if this is a new user (maybe check if a CHANGELOG.md file exists where we expect it to be?) and add a mention to the option in the error message.

If you think this addition might be useful enough I would gladly try to help with a PR

craigmulligan commented 7 years ago

@nazrhom good points.

Just chatted to @lekkas @hedss about new user experience, I'll try and summarize:

What do you think?

nazrhom commented 7 years ago

@craig-mulligan sounds pretty good to me, just two minor observations:

versionist checks if CHANGELOG.md if not it creates one in the root

As this happens after parsing the options, it could create one at whatever location is passed in the config parameter.

versionist checks if the commit history or package.json for a version, if a version exists versionist exits with a helpful message??

I think we should avoid looking at the package.json explicitly. There is two reasons I can think of: first it ties versionist to the node.js environment. Secondly, npm init sets the version to 1.0.0 as default, people probably forget about it, so this might result in a lot of errors on this step. (Plus to get around the error, you can just delete that line from your package.json and run versionist again... which works, but is kinda hacky)

hedss commented 7 years ago

I'm not entirely sure I agree. If there's a package.json then it needs to adhere to the same version scheme as CHANGELOG.md. It's also important, because versionist already deals with this file (in fact if you don't have one, you'll find versionist errors, this needs to be fixed).

As part of the contract work, these two files must be aligned. I am not in favour of any scheme that requires manual intervention.

nazrhom commented 7 years ago

It was my understanding that the only interaction with the package.json comes from the updateVersion hook. In this way versionist doesen't really need to know anything about package.json or nodejs, is this assumption wrong?

lurch commented 7 years ago

(in fact if you don't have one [package.json], you'll find versionist errors, this needs to be fixed).

Isn't that because versionist.conf.js contains updateVersion: 'npm', ? So it should still cause an error if the file is missing, but perhaps it should fail with a nicer error message.

IMHO versionist shouldn't automatically create a CHANGELOG.md if it's missing (which it seems like it does currently?), because the default fromLine: 6 in versionist.conf.js means that versionist wouldn't know what to put as the first 5 lines of the changelog?

Also (in this repo), if I do git checkout 219404723e1 && rm CHANGELOG.md && npm run versionist it seems to add v1.0.0 to the newly-created CHANGELOG.md and change the version in package.json back to 1.0.0, despite there being a perfectly-valid "version": "2.7.0" in package.json previously. Bug? ....Ah, if I instead run git checkout 219404723e1 && rm CHANGELOG.md && npm run versionist -- --current 2.7.0 then the version gets updated to 3.0.0. So maybe versionist should also look at package.json to determine the initial version, if neither getChangelogDocumentedVersions or --current provide anything useful? And/Or maybe it should print some error message/warning if the version in package.json doesn't match what getChangelogDocumentedVersions reports? (Sorry for going OffTopic, let me know if this latter point is worth opening as a separate issue, or if I should just be ignored if I'm misunderstanding how versionist is supposed to work :wink: )

hedss commented 7 years ago

So I've done some checking, and actually I don't believe versionist is at fault. It's an npm package (so by it's nature, it's tied to Node and the concepts of its package management) and it does clearly state that, by default updateVersion defaults to npm.

If you create an empty updateVersion() method in your versionist.conf.js config, that simply calls the callback, then package.json is not checked for and only CHANGELOG.md is updated. This means that components without a package.json can have their own versionist.conf.js with this behaviour. We do not, however, wish to remove that from the default versionist config; see here for why. Only a few components of ours are not JS/CS/TS based, and there are other package management tools out there for alternative scripting languages (and as mentioned here, changing the updateVersion() method could allow for versioning those).

Creating a default CHANGELOG.md is not, in my view, a bad thing as long as it includes the boilerplate including the first 5 lines (as @lurch specified). In fact, this could even be a new option in the versionist.conf.js, where a boilerplate for a new CHANGELOG.md is specified. In this case, again, you'd need to override to change the default. This nicely allows us to automatically produce the right type of file for the config as well as letting users override it if they wish.

@lekkas Anything further you want to bring to the table?

lekkas commented 7 years ago

@hedss @lurch @craig-mulligan @nazrhom reading this thread I realised I'm confused about a core aspect of versionist:

From a versionist perspective, what should be the single source of truth for a project's current semantic version?

Right now we have:

Just to make sure we are on the same page, I believe we all agree (and it is kind of obvious I guess) that all three ^ versions must be aligned after a project is setup to use versionist (and @hedss 's awesome procbots); AFAIK this is something that already happens. Ideally, the tagged commit should be the commit that adds the new CHANGELOG entries and updates manifest files (e.g. package.json for npm projects); AFAIK this happens as well.

Regardless of the current versionist waterfall-like API, I'm against tying versionist to npm projects. The fact that versionist as a tool is shipped as an npm module is irrelevant - it could be shipped with brew or pip. That said, it makes perfect sense to prioritise and facilitate the npm use case (because most resin.io projects are on npm) but versionist should not have a hard requirement of package.json existing.

Let's forget the current versionist state for a moment - @hedss @lurch @craig-mulligan @nazrhom what do you think would be the proper source of truth for a versionist handled project? I have a hard time picking one, I'm tempted to say git tags (maybe this even needs a brief arch call discussion)

I believe we must first conclude this ^ before solving the problem of how to easily initialise a project. Then, we can proceed with @craig-mulligan 's proposed workflow adapted to what versionist considers to be the source of version truth. By the way, given that this step will only be done once, I think it's fair game to ask the user some manual intervention in case there are inconsistencies among git tags / changelog / package.json

(I hope this ^ makes sense)

hedss commented 7 years ago

It's a good question, and my gut feeling is the same as yours; has to be the git tag.

Both the CHANGELOG.md and the package.json (optionally) are there as, respectively, a human and machine parsable files to get what I consider the agreed version. However, whilst these files change, tags are (considered) immutable. Once created, never destroyed.

Given the template idea above, we could get versionist to initialise a new (or existing) repository given simply a tag, with the ability to construct any files with versions embedded in them from it.

lurch commented 7 years ago

Yup, sounds good. And as I suggested earlier, perhaps versionist could warn you if the 'single source of truth' (the git tag) is inconsistent with the 'auxillary versions' (CHANGLEOG.md, and package.json if it's a npm project) - which I guess means there'd need to be a get counterpart of the current updateVersion function (but IMHO simply calling it getVersion might over-emphasise its importance, and imply it's the single-source-of-truth when it isn't, IYSWIM :-/ ). Looking again at @lekkas comments, perhaps we could deprecate the editVersion and updateVersion functions and 'rename' them to editManifestVersion and updateManifestVersion; which would then allow us to easily add getManifestVersion. Effectively making these approximate 'equivalences':

Changelog Manifest
editChangelog editManifestVersion
addEntryToChangelog updateManifestVersion
getChangelogDocumentedVersions getManifestVersion

BTW if we're on the master branch in git, and we've got a bunch of (possibly arbitrary) tags in our history, how easy is it to determine what the correct 'most recent version tag' is? (which obviously impacts how reliable that would be as the single-source-of-truth). I'm sure others here know much more about git than I do :)

If a project chooses not to use git tags (for whatever reason) should we allow the configuration to specify what the 'single source of truth' is, or would that just make things ridiculously over-complicated?

I also agree with "Regardless of the current versionist waterfall-like API, I'm against tying versionist to npm projects." which makes me wonder if it'd be worth setting up e.g. a dummy Python project in resin-io-playground, showing how versionist can be configured for use with non-npm projects? (might also be a good bug-finding exercise?)

lekkas commented 7 years ago

And as I suggested earlier, perhaps versionist could warn you if the 'single source of truth' (the git tag) is inconsistent with the 'auxillary versions' (CHANGLEOG.md, and package.json if it's a npm project)

Agreed that this is the sensible thing to do

Wrt to the API and next versionist steps in general, I'm all in for a cleaner API but at the same time want make sure that any changes/features here will add value to process bots. The line between versionist/versioning process bots is a bit blurry because there's overlapping functionality, so when @hedss finds some peace with the current proof-of-concept procbot work we can get his feedback, talk about it in an arch call, recalibrate and see where we should focus.

If a project chooses not to use git tags (for whatever reason) should we allow the configuration to specify what the 'single source of truth' is, or would that just make things ridiculously over-complicated?

Yeah let's have a hard requirement for git tags for now. With regards to what's the most recent tag, I think we can rely to semver comparison to find it out (versionist requires/assumes semver right?)

I'm also in for testing this on non-node.js projects - basically versionist should be able to create changelog for any version controlled repository (not only code) that uses git, git tags and a specific convention when committing. More than that, what keeps bugging me is how feasible it would be to have a common git commit convention throughout resin.io, regardless of what a repository contains (code [js/coffee/typescript/rust/go] , documentation etc)

lekkas commented 7 years ago

Given the template idea above, we could get versionist to initialise a new (or existing) repository given simply a tag, with the ability to construct any files with versions embedded in them from it.

@hedss sorry I missed this ^ Totally agree.

Hm, how would it handle existing files? Updating package.json is kind of easy because it has a known schema, updating arbitrary changelogs looks hard because we can't assume they follow any standard convention/schema.

lurch commented 7 years ago

With regards to what's the most recent tag, I think we can rely to semver comparison to find it out

But presumably it would/should be possible to have a project with e.g. a 2.x branch and a 3.x branch, with both branches being actively maintained? (which means the 2.x.y and 3.x.y tags will be interspersed) I was wondering if it'd be possible to traverse the parent commits of the current commit, until you hit the most recent version-tag on that branch? (Does that make sense? Is that functionality that git already includes? Or am I simply misunderstanding things?) [ EDIT: Looks like git describe --abbrev=0 2>/dev/null does exactly that :tada: ]

(versionist requires/assumes semver right?)

In theory it's selectable https://github.com/resin-io/versionist#incrementversion-function but I suspect it's never been tested with anything other than semver.

what keeps bugging me is how feasible it would be to have a common git commit convention throughout resin.io, regardless of what a repository contains

I guess that's more of a procbot thing (by blocking commits/PRs that don't match the standard convention) than strictly a versionist thing ;) (I haven't got round to looking at the procbot stuff yet)

updating arbitrary changelogs looks hard because we can't assume they follow any standard convention/schema.

Isn't that exactly what the getChangelogDocumentedVersions and addEntryToChangelog functions are for?

As we haven't already, is it worth getting a couple of quick comments from @jviotti the original versionist author?

hedss commented 7 years ago

But presumably it would/should be possible to have a project with e.g. a 2.x branch and a 3.x branch, with both branches being actively maintained? (which means the 2.x.y and 3.x.y tags will be interspersed)

If multiple branches are going, then only one of these is going to be merged to master anyway. Tags exist on the branch they were created, and do not persist onto another branch. However, if someone was to be on the 2.2 branch and push a major semver version, they're going to essentially try and rewrite the 3.0 tag, which would be baaaad. At the point divergence occurs, the usual accepted practice is to start suffixing the tags, eg 2.2-someCustomer.

Isn't that exactly what the getChangelogDocumentedVersions and addEntryToChangelog functions are for?

To a degree. My preference is that, yes, we just blanket apply our schema to everything resin. This is a discussion waaaay outside the scope of this conversation, I think. A lot of people are going to have a lot of opinions.

lekkas commented 7 years ago

I guess that's more of a procbot thing

Agreed, but just to clarify, I was talking about the human aspect of that change ;) This is something we'd also discussed in the past, that is, if it'd make sense for separate teams/clusters to stick to the same convention (e.g. conventional/angular)

Isn't that exactly what the getChangelogDocumentedVersions and addEntryToChangelog functions are for?

The context here is initialising uninitialised repos with versionist, were a project still doesn't have a known changelog convention

lurch commented 7 years ago

However, if someone was to be on the 2.2 branch and push a major semver version, they're going to essentially try and rewrite the 3.0 tag, which would be baaaad.

True - maybe that's also something else versionist could check for? ;-)

[standard commit conventions]

This is a discussion waaaay outside the scope of this conversation, I think. A lot of people are going to have a lot of opinions.

Oh absolutely - probably something that Alex will have to dictate a hard line on, if or when it happens.

Isn't that exactly what the getChangelogDocumentedVersions and addEntryToChangelog functions are for?

The context here is initialising uninitialised repos with versionist, were a project still doesn't have a known changelog convention

I meant that by editing the versionist config, and supplying their own getChangelogDocumentedVersions function, a user can adapt versionist to whatever changelog convention they happen to be using?