1239629 - Autolander should avoid merge commits if possible

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::GithubBot, defect)

Description

[Mark Banner (:standard8)]

For Hello, we generally like to avoid merge commits if possible, as this gives a cleaner history.

It would be good if autolander could automatically merge PRs with a fast forward of master, where the changeset is already based on the latest master.

Comment 1

[Kevin Grandon :kgrandon]

I agree that it would be nice to support this. The only reason we didn't was that at the time it seemed like we would want to use something similar as 'cherry-pick', which wasn't support by the API too well. If all we need to do is to fast forward the master ref, it shouldn't be too hard to do.

Maybe just start with the non-treeherder flow though. The treeherder testing flow is a bit more complex because it merges everything to an integration branch, and then to master.

Comment 2

[Ed Morley [:emorley]]

For the single-commit case, the new "squash and merge" feature can be (ab-)used to have the same effect as just fast-forward merging:
https://help.github.com/articles/about-pull-request-merge-squashing/

Comment 3

[Ed Morley [:emorley]]

A few questions:
1) The Treeherder integration appears broken/unused (it wasn't converted to use the new Hawk credentials), I'm presuming it and the related taskgraph features can be removed? (There are no mentions of "taskgraph" on papertrail even though debug logging is on)
2) Why does autolander create integration-* branches? (eg see https://github.com/autolander?tab=activity). Is that just for the taskgraph feature and wanting to batch up landings? At the moment it's created even if there isn't a taskgraph.json, when it seems as though that can be skipped if not scheduling jobs.

Many thanks :-)

Comment 4

[Ed Morley [:emorley]]

It looks like the automatic job scheduling on pull requests for eg Gaia is now performed by gaia-taskcluster:
https://github.com/taskcluster/gaia-taskcluster

This confusingly uses the autolander GitHub account, which is why that user makes comments like:
https://github.com/mozilla-b2g/gaia/pull/34382#issuecomment-224477463

...even though that part of the autolander repo code is unused.

Comment 5

[Kevin Grandon :kgrandon]

(In reply to Ed Morley [:emorley] from comment #3)
> A few questions:
> 1) The Treeherder integration appears broken/unused (it wasn't converted to
> use the new Hawk credentials), I'm presuming it and the related taskgraph
> features can be removed? (There are no mentions of "taskgraph" on papertrail
> even though debug logging is on)

I believe this was disabled in gaia due to some flakiness, but it's a very useful feature. It's basically the only way that you can run tests when autolanding to ensure that HEAD is always green. My recommendation would be to actually get this working well and enabled for some repos, instead of deleting the code. I'm not driving this project actively though, so if there are no customers and you don't have time to support it, I'm fine if you want to remove it.

> 2) Why does autolander create integration-* branches? (eg see
> https://github.com/autolander?tab=activity). Is that just for the taskgraph
> feature and wanting to batch up landings? At the moment it's created even if
> there isn't a taskgraph.json, when it seems as though that can be skipped if
> not scheduling jobs.

It creates an integration branch to batch up jobs when submitting taskgraphs. If the treeherder/taskgraph workflow is not active we can probably skip that step.

Comment 6

[Ed Morley [:emorley]]

(In reply to Kevin Grandon :kgrandon from comment #5)
> I believe this was disabled in gaia due to some flakiness, but it's a very
> useful feature. It's basically the only way that you can run tests when
> autolanding to ensure that HEAD is always green.

There are two types of projects that want GitHub automation:
1) Those using Taskcluster to schedule jobs, eg Gaia.
2) Those using Travis/Circle CI/something else already integrated with GitHub

At the moment only projects in group #2 seem to be using the autolander codebase. Gaia is instead using gaia-taskcluster to perform this automation, which has it's own taskgraph support [1] and is in active use at the moment.

I'm wondering as well whether autolander is even the right place for this job-scheduling related code.

ie: I think it makes more sense for Travis/gaia-taskcluster/... to listen for PR changes and schedule jobs, then update the pull request status using GitHub's CI status API. Autolander would then just listen for CI status change events (like Heroku does for "wait until CI passes before deploy") and use that as the basis for deciding whether to merge a PR.

Using GitHub's status API would be particularly handy, since GitHub now allows marking branches as protected, meaning people using taskcluster could block merges unless the CI passed.


[1] https://github.com/taskcluster/gaia-taskcluster/blob/721bfe2f8af9d3c82d0f7f57c25e0f2879e1b6f0/routes/github_pr.js#L175

Comment 7

[Kevin Grandon :kgrandon]

(In reply to Ed Morley [:emorley] from comment #6)
> There are two types of projects that want GitHub automation:
> 1) Those using Taskcluster to schedule jobs, eg Gaia.
> 2) Those using Travis/Circle CI/something else already integrated with GitHub
> 
> At the moment only projects in group #2 seem to be using the autolander
> codebase. Gaia is instead using gaia-taskcluster to perform this automation,
> which has it's own taskgraph support [1] and is in active use at the moment.
> 
> I'm wondering as well whether autolander is even the right place for this
> job-scheduling related code.

If you want to ensure that master is green, autolander is the only correct place where you can schedule these jobs. This is because the base branch can be modified after travis runs on a PR change, where as autolander will always run tests rebased on top of master before landing. I do think you should be running tests in both situations though for developer feedback. Both whenever a PR is updated, and when the dev tries to land a branch.

Comment 8

[Ed Morley [:emorley]]

Autolander (not to be confused with autoland!) has been decommissioned in bug 1419773.