run cpplint with make check?

[mihaibudiu]

I see many Travis runs fail because of cpplint. Maybe we should consider running it together with make check.

[jnfoster]

+1

[sethfowler]

It's definitely easy to forget to run it.

[mihaibudiu]

Anyone volunteer to do this?
Speaking of build, it looks like our process for merging into master is bottlenecked by the need to always merge with the latest and wait for Travis. There must be a better way to do this. We have had approved PRs waiting more than a week to be merged. Also, we don't know if someone else is working on merging them, so there is a race where we trigger needless merges with master.
In general, I have tried to avoid merging my own PRs, but I sometimes step up to do it if they were approved. Maybe we can add a label to the PR, something like "next to be merged"?

[jnfoster]

I think we need a merge queue.

…

On Wed, May 24, 2017 at 10:25 AM, Mihai Budiu ***@***.***> wrote: Anyone volunteer to do this? Speaking of build, it looks like our process for merging into master is bottlenecked by the need to always merge with the latest and wait for Travis. There must be a better way to do this. We have had approved PRs waiting more than a week to be merged. Also, we don't know if someone else is working on merging them, so there is a race where we trigger needless merges with master. In general, I have tried to avoid merging my own PRs, but I sometimes step up to do it if they were approved. Maybe we can add a label to the PR, something like "next to be merged"? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#619 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABwi0k8iue9HMTodarjn3lSpLd33DhOpks5r9Gf-gaJpZM4NZiQ9> .

[antoninbas]

Can't you just disable "Require branches to be up to date before merging" for this repo?
I agree that this has been a major pain, I keep having to click the "Update" button, and then I forget to merge and have to start again 2 hours later...

[sethfowler]

It is definitely a pain.

The downside of allowing people to merge un-rebased changes is that they may have been broken by another commit that landed in the meantime. In my experience that happens quite frequently for non-trivial patches.

One approach that is used to solve this problem is to merge PRs to an integration branch (dev or something similar) and manually merge commits from that branch to master once we're confident that the tests pass. That is effectively a merge queue.

If we used that approach, someone would need to take responsibility for managing the integration branch. Doing things this way has the effect of reducing the effort expended by most team members at the cost of greatly increasing the amount of work for whoever manages that branch, since they'll have to do things like manually back broken patches out of the integration branch.

I don't think that approach is a good fit for this project, really.

A better idea would be to set up a bot that auto-rebases patches and auto-merges reviewed patches once they pass the tests. People have written a lot of bots that do this, including bors, which I've used before on other projects, and homu, which looks great, though I haven't used it before.