LGTM is a simple pull request approval system for GitHub | Hacker News Reader

LGTM is a simple pull request approval system for GitHub

github.com

136 pointssamber58 comments

I first encountered LGTM on some of Airbnb's open source repos, here's an example of it at work on a closed [1] and open [2] Enzyme PR:

1: https://github.com/airbnb/enzyme/pull/449

2: https://github.com/airbnb/enzyme/pull/459

I love that this enforces a social practice a lot of teams use already, and I'm pushing to add it to my own team's workflow (both internal and for open source contributions).

Edit: another tool in the same vein people might be interested in is Facebook's mention-bot [3], which analyzes PR diffs and @-mentions people it thinks would be good reviewers based on git blame of lines touched.

3: https://github.com/facebook/mention-bot

Another tool is https://github.com/zalando/zappr . We built this and we are currently quite happy with it and open to suggestions, issues, PRs.

Why is this being down voted?

Here's a blog post on Zappr explaining its origins and more about how it works: https://tech.zalando.de/blog/zappr--enhancing-your-github-wo...

http://homu.io goes a bit further (it actually triggers CI builds of the merge results to check they are okay), but it seems it is currently unmaintained.

BTW, I would gladly pay money for this! Even more in combination with features from Facebook's mention-bot/Rust's highfive.

> http://homu.io goes a bit further (it actually triggers CI builds of the merge results to check they are okay), but it seems it is currently unmaintained.

barosl apparently hasn't had the time to work on it, and homu-as-a-service has apparently stopped working some time ago. After discussing it with barosl Rust has apparently moved to the Servo fork of it: https://github.com/servo/homu

No bors/homu as a service at this point though, sadly.

https://github.com/rust-community/team/issues/41

https://github.com/barosl/homu/issues/122

https://github.com/barosl/homu/issues/151

There are two pull requests we are working to merge that might be of interest. First is the ability to auto-merge once all approvals are received. Second is the ability to auto-tag a release once all approvals are received.

Regarding CI, this generally works in conjunction with a CI server. You can specify, via github protected branches configuration, that both LGTM and your CI checks must pass before merging.

https://b.agilob.net/choose-gitlab-for-your-next-project Point 5.

IIRC gitlab doesn't queue PRs (so you could potentially get a failing master branch), but yeah, that covers most use cases.

agilob.net is down, probably this feature is referenced https://about.gitlab.com/2015/07/29/feature-highlight-merge-...

Regarding PR queuing do you mean https://gitlab.com/gitlab-org/gitlab-ce/issues/4176 ?

Basically yes.

Cool, https://gitlab.com/gitlab-org/gitlab-ce/issues/4176 has our attention but it doesn't have a milestone yet. Feel free to leave a comment there with your use case and how you think it should look.

Commented, thanks!

I cannot find a way to "undo LGTM" if I was to quick to give approval. Working at a company where I have to LGTM commits literally every day, believe me that it is necessary every now and then.

In zappr (https://github.com/zalando/zappr) you can, check it out. An example: https://github.com/zalando/zappr/blob/master/.zappr-example.... .

it would definitely be trivial to add the ability to rescind an approval with a -1, for example. Unfortunately I'm a bit bogged down with my other OSS projects at this time, but I would be happy to accept a PR. In fact, this is the function you would want to update https://github.com/lgtmco/lgtm/blob/master/web/hook.go#L100

Not talking about the project, but is the term 'LGTM' (looks good to me) used often in tech? I've heard it from Googlers but otherwise haven't really seen the term in tech.

Looks Good to Me is commonly used in code review at my employer (not Google).

Same here we are using it for most of our commits. When someone is done with their changes, they issue a PR which is reviewed by one or two team members before being rejected or merged by saying LGTM.

It's been used at companies I've worked at (not Google).

I had a coworker who didn't know what it meant, and somehow assumed it was "Let's Get That Money". So now I think of that every time :)

For a long time, I actually thought it means "Let's Get That Moving". Still works.

Same here, for the same reason. Apparently that's a 50 Cent track - go fig!

Depends on the company, but at SendGrid it's standard practice in the code review stage to wait for Looks Good To Me - no more changes needed - before merging.

Much like this submission, our continuous integration system (opsbot) doesn't let you continue through the workflow until you have a "LGTM" comment from another developer. This prevents lone-developer cowboy merges with no review. :)

I had assumed it was very common. The small start up I worked at used it. One of the developers was really fond of using this plugin for all his LGTM comments

https://chrome.google.com/webstore/detail/lgtm/oeacdmeoegfag...

I've always had it as "Looks Good To Merge" -- we use it every day...

it's used more generally than in code. eg design doc review you'd say "LGTM" to say that there are no suggestions.

Let's Get The Manual

Let's get this merged!

Yup, its used often. Atleast in my company(not Google), LGTM and SGTM are used pretty commonly.

I think I picked it up from looking at Angular PRs but have spread it to my team so now we all use it.

I also heard Google uses it, in none of my previous employees I've seen it (corporations).

It's common at Microsoft to sign off on a code review with "LGTM".

Google uses LGTM quite heavily in UI labels and conversations, e.g. "I LGTMed it".

Another option is to do away with blocking on pull requests altogether and review after the fact. It has some benefits (and some downsides).

I blogged about that here: http://thepugautomatic.com/2014/02/code-review/

We use a home-grown tool on top of GitHub to review commit-by-commit after the fact: https://github.com/henrik/remit

Is this type of functionality planned for an official GitHub feature? Gitlab and Bitbucket both incorporate some type of similar functionality already...

We use github-review-helper instead https://github.com/salemove/github-review-helper . It also has fixup commits support which is quite useful in our workflow.

Since we're talking about GitHub pull requests, my favorite tool for this is https://reviewable.io/. It fixes basically everything about GitHub pull requests.

And of relevance to the original article, in Reviewable you can customize your review completion condition with a snippet of code it will run on your behalf and post the result as a GitHub commit status, so you can have basically any approval system you want. One of the examples is a simple LGTM-based system like in OP.

(Disclaimer: I run reviewable.io.)

I also recommend checking out PullApprove, we've had some success using it in some of our open source projects: https://pullapprove.com/

Do you know if there is the same thing in Gitlab?

Definitely not as feature-filled as the post, but +1 and thumb ups will change the view to show people have approved a merge request.

Works well for the team I'm on.

yes, I know, but my company is using Github for a long time and migrate is not planed.

You should compare annual costs of GitHub and GitLab CE or EE and start planning it.

Also on bitbucket...

Hmm, I mostly work with Stash nowadays and they have the approve/disapprove buttons which I like better than LGTM comment.

I'm a bit late to the party here. I'm the author of the project and happy to answer any questions.

The Kubernetes submit-queue munger is also worth a look: https://github.com/kubernetes/contrib/tree/master/mungegithu...

I really appreciate when project authors make it so simple for users to deploy. Thank you, folks!

Does anyone have experience with https://github.com/reviewninja/review.ninja

Why not make it distributed? Git is distributed

How would you make that distributed? You still need a unique repository that serves as a reference and on which pull-requests are made.

yes, or a unique branch

It would be nice if instead of having to comment LGTM a thumbs up reaction on the FP of the PR would suffice

LGTM doesn't currently work with GitHub reactions mostly because the API [1] wasn't available back in November when the project was created. I do personally like comments because they are a bit more visible than reactions, which require you to hover over to see more details.

Now that the reactions API is available it is certainly possible to enable this sort of functionality. I openly encourage people to fork the project and hack into something that works for their team.

[1] https://developer.github.com/changes/2016-05-12-reactions-ap...

I believe the actual "LGTM" phrase is configurable via a .lgtm file in the root of the repo. Airbnb's Enzyme has it configured to work with a :+1:.

From reading the manual I saw it was configurable to change the regexp it matches, not property's of the OP

Is it:

"Looks Good to Me"

"Let's Get this Moving"?

:-)

Let's Git This Merged? :D

Reply on news.ycombinator.com