Node: A bookmarklet to check commit messages in PRs

[vsemozhetbyt]

I am not sure if this is a proper place to share this small thing, feel free to close and redirect)

Currently, we have at least two very good tools for collaborators: core-validate-commit and node-review. However, they are not convenient to check PR commit messages in GitHub web interface. So I've jotted down a little silly bookmarklet for this.

To save it, you can simply select the code (from javascript: { up to the last }) and drag it to the bookmarks panel, then edit the bookmark name.

To use it, you should be in the 'Conversation' or 'Commits' tab of a PR (the latter is neater as it uses only own PR commits, not commits from other PR/issues cross-references).

It checks the most formal commit guidelines rules: title/lines length, title format, full URLs. It uses title attribute of commit links. It marks these links according to the check result and adds a small red ! sign near erroneous commits with a title attribute containing error explanations. It also alerts the overall check result and scrolls the page up to the first commit.

To test it just open any PR with many commits (the last example) and click on the bookmarklet.

It can produce many false positive (or false negative) messages for now.

Feel free to fork and adjust anything)

[vsemozhetbyt]

Now all error messages are outputted to the console (for an easier copy-pasting).

[gibfahn]

This is really nice @vsemozhetbyt , I'm a big fan of the red/green highlighting.

Maybe this is something you could add into node-review (cc/ @evanlucas)? That way when you click the button you get both. If you did that it'd be easier for people to update to a newer version (just a git pull in the cloned repo).

[vsemozhetbyt]

@gibfahn I am a bit uncertain how to coordinate the logics of the both tools. This bookmarklet mostly aims initial commits to fix nits early. node-review comes on the scene in the final act: it aborts any actions if PR is not fully approved. So maybe it would be more convenient for @evanlucas to embed any trifles he will consider useful if they fit well :)

[gibfahn]

I know node-review doesn't show anything if there haven't been any approvals, but that doesn't stop you checking the commit messages anyway right?

Is there a reason you couldn't have the node-review button show the approver information and also check the commits? We could even have logic that automatically puts a corrected commit message in the blue box at the top.

I agree your bookmarklet and node-review currently do different things, but they both have the goal of making Collaborators' lives easier, so maybe they could be part of the same tool.

[vsemozhetbyt]

@gibfahn I shall try to make a PR for node-review after learning its code.

Meanwhile, I've refactored the bookmarklet so it can be used as a user script for Tampermonkey. I've removed autoscrolling, replased alerts by a small HTML element and added observing for GitHub internal navigation. So now this code can be plugged in as a user script for autochecking all node PR commits without manually clicking on bookmarklet (while it still can work as a bookmarklet as well).

[vsemozhetbyt]

Console output is replaced by a possibility to copy the full log to clipboard (by clicking on the overall results info).

It seems the node-review will be significantly refactored soon. Maybe it is better to postpone additions till that PR landed.

[vsemozhetbyt]

@gibfahn It seems there is a plan to implement this linting as a bot script: https://github.com/nodejs/github-bot/issues/54 It's a pity this plan is a bit stalled.

[vsemozhetbyt]

Wow.
https://twitter.com/evanhlucas/status/851462701754834944
https://ci.nodejs.org/job/node-test-commitmsg/

[kfarnung]

It seems the node-review will be significantly refactored soon. Maybe it is better to postpone additions till that PR landed.

@vsemozhetbyt That PR doesn't speak for @evanlucas future plans, I just wanted to see what it would take to update the extension to be compatible with Edge and Firefox as well as Chrome. The changes there are extensive, but can be broken out as necessary to land the PR if people are interested in the changes, but no comments yet.

TLDR: I wouldn't let my change block you from making improvements, I definitely think they would be useful.

[vsemozhetbyt]

@kfarnung Unfortunately, I know almost nothing about Chrome Extensions. I meant if I had time to learn the theory and to examine the node-review code, it would be better to deal with the already refactored code. However, I really appreciate your feedback and I will take it into consideration)

[kfarnung]

Ah, OK. They aren't too hard to pick up the basics, most of what your bookmarklet does should wind up in the content script portion of the extension. As long as it doesn't take a dependency on the script environment (variables of the page's script), then you should be fine. You'll have full access to the DOM for reading and manipulation.