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

Split changelog into files by version #3284

Merged
merged 4 commits into from
Aug 16, 2016
Merged

Conversation

jewel-andraia
Copy link
Collaborator

@jewel-andraia jewel-andraia commented Aug 16, 2016

and make version changelog file from changelog/UNRELEASED.md when running npm version.

⚠️ fails awkwardly if changelog/vNew.Version.Number.md already exists.

const unreleasedChangelog = 'changelog/UNRELEASED.md';
const templateChangelog = 'changelog/_EXAMPLE.md';

const version = metadata.version;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe check if newChangelog exists first?

@Ajedi32
Copy link
Contributor

Ajedi32 commented Aug 16, 2016

I guess this is to make it easier to automate maintenance of the change log? That's pretty cool. Worth noting though that there are a couple minor disadvantages to this approach:

  1. It makes it more difficult to Ctrl+F through the CHANGELOG to find out in what version of RES a specific change was implemented.
  2. The sort order of the versions in changelog/ won't necessarily be oldest to newest. (E.g. A theoretical 4.7.10 would be sorted before 4.7.9 if you're sorting by name.)
  3. It's marginally less discoverable, since you're no longer using the conventional CHANGELOG file in the root of the repo.

These are all definitely solvable problems though:

  1. We could include the full, concatinated CHANGELOG in a page on http://redditenhancementsuite.com/
  2. No real solution, but the solution for 1 above also lessens the impact of this.
  3. We could keep a CHANGELOG.md file in the root that tells people to look in changelog/

@jewel-andraia
Copy link
Collaborator Author

We could include the full, concatenated CHANGELOG in a page on http://redditenhancementsuite.com/

I like the sound of that idea. I set up the known issues page on the upcoming res.com in a similar fashion: http://reddit-enhancement-suite.github.io/knownissues/

We could keep a CHANGELOG.md file in the root that tells people to look in changelog/

Good idea! And a pointer to res.com, too.

@@ -27,7 +29,8 @@
"lint-fix": "eslint . --fix",
"test": "cross-env NODE_ENV=test ava",
"coverage": "cross-env NODE_ENV=test nyc ava",
"report-coverage": "nyc report --reporter=text-lcov | coveralls"
"report-coverage": "nyc report --reporter=text-lcov | coveralls",
"version": "babel-node build/version.js"
Copy link
Contributor

@Ajedi32 Ajedi32 Aug 16, 2016

Choose a reason for hiding this comment

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

Might want to use postversion for this, so that build/version.js isn't using the old version when creating the changelog file. Although, I guess that might mess up the tagging? Hmm... does this work the way I'm imagining it does?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, version is what we want to use. preversion has the old version number, version has the new version number and is before the commit (which we want to add to), and postversion is after the commit

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, really? Somebody needs to tell npm to update their docs:

preversion, version: Run BEFORE bump the package version

😜

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh. Haha. They just did: npm/npm@c565d89

Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh, I'm surprised that's not deployed yet (I guess only on a new npm release?).

The npm version docs go into more detail and seem to be correct:

  1. Run the version script. These scripts have access to the new version in package.json (so they can incorporate it into file headers in generated files for example). Again, scripts should explicitly add generated files to the commit using git add.

@@ -0,0 +1,30 @@
/* eslint-disable import/no-commonjs, import/no-nodejs-modules */
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess import/no-commonjs can be removed now since you're using ES6 imports?

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.

None yet

3 participants