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

Make changelog linting more effective #701

Closed
kumar303 opened this issue Dec 21, 2016 · 4 comments
Closed

Make changelog linting more effective #701

kumar303 opened this issue Dec 21, 2016 · 4 comments

Comments

@kumar303
Copy link
Contributor

Is this a bug or feature request?

code quality enhancement

What is the current behavior?

When you develop locally or create a PR, you have to follow the commit message format. If you make a mistake, you'll get a failure locally. If you push a bad commit to your branch, you'll see failures forever unless you rewrite the commit history.

What is the expected or desired behavior?

This isn't very effective because ultimately a maintainer will squash and merge the PR to master, choosing a new commit message anyway. There is no linting for this message! As a maintainer myself, I also like to spend time on this final squashed message since it will go into the auto-generated changelog.

We should come up with a more effective linting strategy.

The first action items:

  • remove the local pre-commit hooks
  • remove the TravisCI test for changelog linting

Next, we need a new linting approach. Ideas:

  • create a bot that watches for new PRs and pings @kumar303 or @rpl in IRC if a title needs fixing. PR titles are used by git to create the squashed commit message.
  • create a webextension for maintainers that warns if the text box has an improperly formmated message when doing a squash-on-merge.
@kumar303
Copy link
Contributor Author

@rpl maybe we don't need a bot at all. Another idea: make the CI test look for TRAVIS_PULL_REQUEST, check its title (using the github api?), and run it through the changelog linter.

@rpl
Copy link
Member

rpl commented Dec 22, 2016

@kumar303 👍 yeah, even better, I like it!

we can use the above environment var and request the JSON description of the pull request, e.g. for #677 from the API URL https://api.github.com/repos/mozilla/web-ext/pulls/677 we get:

{
  ...
  "title": "Update grunt-webpack to the latest version 🚀",
  ...
}

@kumar303
Copy link
Contributor Author

One thing to keep in mind about pull requests like the Greenkeeper ones is that if there is only one commit in the pull request, GitHub will use the lone commit message as the title for the merge commit. In other words, the title for the grunt-webpack upgrade is technically not formatted right but GitHub sets the merge commit to this which is formatted correctly:

chore(package): update grunt-webpack to version 2.0.0-beta.4 (#677)

@kumar303
Copy link
Contributor Author

I actually requested to bring the title in line with the format (greenkeeperio/greenkeeper#343) because it's not uncommon to pile on multiple commits to fix breakage from an upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants