Repository navigation
A bookmarklet to check commit messages in PRs #12107
Description
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)
Activity
vsemozhetbyt commentedon Mar 30, 2017
Now all error messages are outputted to the console (for an easier copy-pasting).
gibfahn commentedon Mar 31, 2017
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 agit pullin the cloned repo).vsemozhetbyt commentedon Mar 31, 2017
@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-reviewcomes 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 commentedon Mar 31, 2017
I know
node-reviewdoesn'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-reviewbutton 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-reviewcurrently 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 commentedon Apr 1, 2017
@gibfahn I shall try to make a PR for
node-reviewafter 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
nodePR commits without manually clicking on bookmarklet (while it still can work as a bookmarklet as well).vsemozhetbyt commentedon Apr 1, 2017
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-reviewwill be significantly refactored soon. Maybe it is better to postpone additions till that PR landed.vsemozhetbyt commentedon Apr 2, 2017
@gibfahn It seems there is a plan to implement this linting as a bot script: nodejs/github-bot#54 It's a pity this plan is a bit stalled.
vsemozhetbyt commentedon Apr 10, 2017
Wow.
https://twitter.com/evanhlucas/status/851462701754834944
https://ci.nodejs.org/job/node-test-commitmsg/
kfarnung commentedon Jun 19, 2017
@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 commentedon Jun 20, 2017
@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 commentedon Jun 20, 2017
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.