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

A bookmarklet to check commit messages in PRs #12107

Closed
vsemozhetbyt opened this issue Mar 29, 2017 · 11 comments
Closed

A bookmarklet to check commit messages in PRs #12107

vsemozhetbyt opened this issue Mar 29, 2017 · 11 comments
Labels
meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Mar 29, 2017

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 vsemozhetbyt added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Mar 29, 2017
@vsemozhetbyt
Copy link
Contributor Author

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

@gibfahn
Copy link
Member

gibfahn commented 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 a git pull in the cloned repo).

@vsemozhetbyt
Copy link
Contributor Author

@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
Copy link
Member

gibfahn commented Mar 31, 2017

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
Copy link
Contributor Author

vsemozhetbyt commented Apr 1, 2017

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@kfarnung
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor

kfarnung commented 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

No branches or pull requests

3 participants