HN Explorer

61 comments

[ 0.28 ms ] story [ 127 ms ] thread
xgbi
Do you know if there is the same thing in Gitlab?
samber
yes, I know, but my company is using Github for a long time and migrate is not planed.
akerro
You should compare annual costs of GitHub and GitLab CE or EE and start planning it.
exception_e
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.

[deleted]
(comment deleted)
bjacobel
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

kerneis
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.
killercup
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.

masklinn
> 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

akerro
https://b.agilob.net/choose-gitlab-for-your-next-project Point 5.
Manishearth
IIRC gitlab doesn't queue PRs (so you could potentially get a failing master branch), but yeah, that covers most use cases.
sytse
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 ?

bradrydzewski
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.

cvburgess
Is this type of functionality planned for an official GitHub feature? Gitlab and Bitbucket both incorporate some type of similar functionality already...
x0rg
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.
ninjakeyboard
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.
llamataboot
I've always had it as "Looks Good To Merge" -- we use it every day...
ninjakeyboard
it's used more generally than in code. eg design doc review you'd say "LGTM" to say that there are no suggestions.
mdk754
Let's get this merged!
loeg
Looks Good to Me is commonly used in code review at my employer (not Google).
Kunlun
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.
[dead] Kunlun
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.
colbyaley
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 :)

throwanem
Same here, for the same reason. Apparently that's a 50 Cent track - go fig!
realityking
For a long time, I actually thought it means "Let's Get That Moving". Still works.
etjossem
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. :)

krzyk
I also heard Google uses it, in none of my previous employees I've seen it (corporations).
bubersson
Google uses LGTM quite heavily in UI labels and conversations, e.g. "I LGTMed it".
joshschreuder
I think I picked it up from looking at Angular PRs but have spread it to my team so now we all use it.
reubenbond
It's common at Microsoft to sign off on a code review with "LGTM".
agnivade
Yup, its used often. Atleast in my company(not Google), LGTM and SGTM are used pretty commonly.
glibgil
Why not make it distributed? Git is distributed
hk__2
How would you make that distributed? You still need a unique repository that serves as a reference and on which pull-requests are made.
glibgil
yes, or a unique branch
fishnchips
I really appreciate when project authors make it so simple for users to deploy. Thank you, folks!
johnwheeler
Is it:

"Looks Good to Me"

or

"Let's Get this Moving"?

:-)

silasb
Hmm, I mostly work with Stash nowadays and they have the approve/disapprove buttons which I like better than LGTM comment.
eridius
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.
piotrkaminski
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.)

PuercoPop
It would be nice if instead of having to comment LGTM a thumbs up reaction on the FP of the PR would suffice
bjacobel
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:.
PuercoPop
From reading the manual I saw it was configurable to change the regexp it matches, not property's of the OP
bradrydzewski
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...

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