Skip to content

init fails when calculated version equal to the current version - #11

Merged
daveashworth merged 5 commits into
masterfrom
11-init-fails-on-equal-versions
Dec 5, 2016
Merged

daveashworth merged 5 commits into
masterfrom
11-init-fails-on-equal-versions

Conversation

@daveashworth

Copy link
Copy Markdown
Collaborator

Currently we get an error when --init is run and the calculated version == current version. It should be allowed that the versions can be equal on this initial run.

@daveashworth daveashworth self-assigned this Nov 17, 2016
@daveashworth daveashworth changed the title init flag should be ok with the calculated version being equal to the current version init fails when calculated version equal to the current version Nov 17, 2016
@daveashworth

daveashworth commented Nov 17, 2016 •

Copy link
Copy Markdown
Collaborator Author

With the following background information:

--init generates a completely new CHANGELOG from scratch and in the process, also calculates the current version of the repo. When pushed (--push) upstream, a npm version {generated version} command takes place to set the correct version in package.json. Trouble comes when --init is run and the generated version is the same as what's already in the codebase and published to npm.

We need to consider this scenario:

  • A repo doesn't currently have a CHANGELOG
  • Running --init generates the CHANGELOG and calculates the version of the repo the same as what's listed in version of package.json
  • Adding --push and --publish, the changes get pushed to Github, but we can't really publish to NPM without bumping the version. (See Here)

Essentially we're adding a CHANGELOG.md to the repo, but we're not bumping the version. Maybe this should/has to be a patch release in this case?

@comamitc

Copy link
Copy Markdown

Here's a SS just in case.

screen shot 2016-11-17 at 4 03 19 pm

@ericclemmons

Copy link
Copy Markdown
Owner

@daveashworth My use-case for GSV would be (for existing projects):

  • Running --init & updating PRs until the versions match up & the CHANGELOG.md that's never existed is generated. I'd expect to arrive at the same version number.
  • However, wouldn't adding GSV to a new repo warrant a commit with the new dependency, updated package.json scripts, etc.?
  • So maybe the documented behavior for an existing repo is running the appropriate commands to confirm it's at parity, then commit the new release scripts & let CI do the version bump (which would be a patch) automatically?

@daveashworth

Copy link
Copy Markdown
Collaborator Author

This scenario is a bit tricky to handle automatically. The addition (or refresh) of a changelog in this scenario has to be accounted for in the changelog history itself since the repo version has to be bumped in order to push or publish.

Initial thought was to give the user a choice on how they wanted to bump the version (patch, minor, major), but without the ability to label a commit with the version change (just PRs), future changelog generations would be out of sync. That commit would always be treated as a patch.

That lead to the thought of making a separate commit for the new changelog with a message like, "CHANGELOG refresh", and then basically running --bump --changelog internally to account for the version bump and changelog addition in the history. That works fine unless CI is already setup to run on commit. In this case you can't mark the commit with [ci skip] because the changelog generation ignores those when generating the changelog, and we want this history reflected in the changelog. We could mark this commit with another special tag in the message, but that feels a little dirty.

@daveashworth

daveashworth commented Nov 29, 2016 •

Copy link
Copy Markdown
Collaborator Author

So @ericclemmons, maybe in the scenario where the versions are equal (adjusting PRs until versions match up, or they do initially), --push and --publish wouldn't be allowed when running --init. A manual commit to master (or through a PR) would need to be done, and then the new scripts would handle bumping the changelog from there (I think that's the same as what you suggested above, just disallowing the use of --push and --publish when using --init).

@ericclemmons

Copy link
Copy Markdown
Owner

I see where I was confused.

Based on the docs, I expected that --init was a bit like --help commands, in that it's one of the first logic blocks to run & bails, preventing any further actions.

Explicitly throwing if other args are provided would be useful, but I'm cool with just bailing if someone does --init --push --bump --fsck if that's much easier.

@daveashworth daveashworth changed the title init fails when calculated version equal to the current version init fails when calculated version equal to the current version [DON'T MERGE] Nov 30, 2016
@daveashworth daveashworth changed the title init fails when calculated version equal to the current version [DON'T MERGE] init fails when calculated version equal to the current version Nov 30, 2016
@daveashworth

Copy link
Copy Markdown
Collaborator Author

I think I'm happier with this now. If gsv comes up with version parity on --init, it's left to the user to commit the changes manually.

I'm also using ora to give the user better feedback on what's happening in the background when gsv is run on the command line.

Should be good to go @ericclemmons.

Comment thread src/debug.js
import debug from "debug";

debug.enable("github-semantic-version:*");

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The one question I do have is since debug.js is imported across multiple files, I couldn't think of a great way to enable/disable based on the --debug flag. I removed this line and left it up to using the DEBUG=github-semantic-version:* environment variable before running the app. Is there a better way?

@daveashworth

Copy link
Copy Markdown
Collaborator Author

Successful changelog generation and version calculation on --init:
platform_calculating_2

Version parity on --init:
platform_calculating

@daveashworth daveashworth mentioned this pull request Dec 5, 2016
@ericclemmons

Copy link
Copy Markdown
Owner

Man, that's some legit output! 👏

@daveashworth

Copy link
Copy Markdown
Collaborator Author

You good with me merging this now @ericclemmons ?

@ericclemmons

ericclemmons commented Dec 5, 2016 via email

Copy link
Copy Markdown
Owner

@daveashworth
daveashworth merged commit 2625d27 into master Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants