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

chore: update CHANGELOG automation to use conventional-changelog #3669

Merged
merged 3 commits into from
Oct 6, 2016

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Oct 5, 2016

Description

This updates our changelog automation to use conventinoal changelog. Ultimately, it will make it easier to merge PRs because it won't require the use of the contrib tool.
It uses the videojs preset

Specific Changes proposed

  • Remove the header from CHANGELOG.md
  • use shelljs-nodecli to execute conventional-changelog -p videojs which prints out the current changelog item.
  • add changelog npm script to re-generate the changelog
  • use npm run changelog in contrib.json for releases

@gkatsev gkatsev added this to the 5.12 milestone Oct 5, 2016
@gkatsev
Copy link
Member Author

gkatsev commented Oct 5, 2016

Output looks like the following.


5.12.3 (2016-10-05)

Features

  • lang: add missing translations in fr.json (280ecd4)
  • lang: add missing translations to el.json (eb0efd4)

Bug Fixes

  • controls: fix load progress bar never highlighting first buffered time range (ca02298)
  • css: remove commented out css (5fdcd46), closes #3587
  • disable HLS hack on Firefox for Android (#3586) (dd2aff0)
  • proxy ios webkit events into fullscreenchange (#3644) (e479f8c)
  • html5: disable manual timeupdate events on html5 tech (#3656) (920c54a)

Chores

  • move metadata to hidden folder and update references (86f0830)
  • deps: add the bundle-collapser browserify plugin (816291e)
  • package: remove es2015-loose since it's an option for es2015 (#3629) (c545acd)
  • package: update grunt-contrib-cssmin to version 1.0.2 (#3595) (54e3db5)
  • package: update grunt-shell to version 2.0.0 (#3642) (2032b17)
  • refactor redundant code in html5 tech (#3593) (6878c21)
  • refactor redundant or verbose code in player.js (#3597) (ae3e277)
  • update object.assign to ^4.0.4 (08c7f4e)

Documentation

  • fix broken links in docs index.md (4063f96)

Tests

  • a11y: add basic accessibility testing using grunt-accessibility (7d85f27)

@misteroneill
Copy link
Member

Looks great! 👍

@@ -31,7 +32,7 @@ module.exports = function(grunt) {
release: {
tag_name: 'v'+ version.full,
name: version.full,
body: chg.find(version.full).changesRaw
body: nodeCli.exec('conventional-changelog', '-p videojs', {silent: true}).output
Copy link
Contributor

Choose a reason for hiding this comment

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

im not sure its worth using shelljs-nodecli for this, shouldn't we just use shelljs and point to path.join(__dirname, '..', 'node_modules', '.bin', 'conventional-changelog'). My reasoning here is that shelljs-nodecli is not very heavily used, spin off modules are usually not kept up to date with the parent, and I don't even see a git repo for it on npm.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a valid concern. The repo is available here, I think it's just missing from the package.json. Also, nicholas has a good track record of maintaining modules or giving it away to other maintainers to help out (see eslint as an example). In addition, I think at this point shelljs-nodecli is feature complete and there isn't much necessary to do with it; for example, it hasn't been updated in 3 years but still works great.
In addition, it will instead encapsulate the whole thing of running a node binary easily, so, we don't have to worry about how to run it properly or get to the binary, we only need to think about how to use conventional-changelog.
Hopefully, this addresses your concerns.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that works then

Copy link
Contributor

@brandonocasey brandonocasey left a comment

Choose a reason for hiding this comment

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

LGTM

@gkatsev gkatsev merged commit d4e89d2 into videojs:master Oct 6, 2016
@gkatsev gkatsev deleted the changelog branch October 6, 2016 18:09
@hartman
Copy link
Contributor

hartman commented Oct 18, 2016

Hmm, I just got:

video.js djhartman$ grunt dist
Loading "Gruntfile.js" tasks...ERROR
>> Error: Couldn't find conventional-changelog.
Warning: Task "dist" not found. Use --force to continue.

After this change. Probably because i don't have ./node_modules/.bin on my path.

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.

4 participants