[GHC DevOps Group] CircleCI job accounting question

[Simon Peyton Jones]

The problem is that many contributors, including Simon PJ, Richard, and
me, tend to push batches of work

I have not been following this thread (“job accounting” seemed above my pay grade) but I saw this mention of my name 😊.   Without having read myself into the context there seem to be two issues


  *   Every commit to master should be validate-clean, and this should be tested by the CI framework not by the contributor. This is essential.   I would be delighted if every commit I made went through that gate. I’m careful, but occasionally not careful enough.

  *   Most – perhaps all – commits should go through a code-review process.  Here I freely admit that I tend to use (or abuse?) my status to make most of my commits without review, except perhaps informally with individuals.  I’d be absolutely willing to review this if (a) in fact people think that the extra step would really improve quality (perhaps looking at past commits) or (b) the very fact that I do so makes people feel cross.


In both cases, the big thing from my point of view is that, once I’m ready to press “go”, I’d like to take it off my to-do list by pushing into a queue that will result in either

  *   a commit to master, or
  *   an email to me saying “more work to do here”

I’m sure that many contributors would value this property.  The tricky bit is, I suppose, deciding when the reviewers are happy.  Maybe that need central human intervention; I’m not sure.

Also important: I often accumulate a sequence of related patches:


  *   I’d like to be able to queue them as a group, or at least with very low additional overhead per patch



  *   Sometimes I have attempted to separate two things (e.g. a bug fix and a refactoring).  In the past I have not felt it important to guarantee a validate-clean state between the two; e.g. I might have erred in separating the two.  The cost benefit ratio of teasing them apart has not seemed worth the pain.  But separating them at least means we have two commit messages.



On this point I can see that I might have to change my behaviour, because the CI would be obliged to stop as soon as it found one that didn’t validate.  I might need help to figure out how best to fix up a patch sequence by moving little bits from one to another, but I’m sure it’s possible.

I’m very keen to support a better CI process in any way I can.

Simon

From: Ghc-devops-group [mailto:ghc-devops-group-bounces at haskell.org] On Behalf Of Manuel M T Chakravarty
Sent: 11 December 2017 06:17
To: Ben Gamari <ben at well-typed.com>
Cc: ghc-devops-group at haskell.org; Mateusz Kowalczyk <fuuzetsu at fuuzetsu.co.uk>
Subject: Re: [GHC DevOps Group] CircleCI job accounting question

23.11.2017 06:15 schrieb Ben Gamari <ben at well-typed.com<mailto:ben at well-typed.com>>:
Simon Marlow <marlowsd at gmail.com<mailto:marlowsd at gmail.com>> writes:


On 22 November 2017 at 15:31, Ben Gamari <ben at well-typed.com<mailto:ben at well-typed.com>> wrote:


Manuel M T Chakravarty <manuel.chakravarty at tweag.io<mailto:manuel.chakravarty at tweag.io>> writes:

Why do we need the intermediate builds exactly? Wouldn’t they usually
fail? (When I do PRs with multiple commits, the state of the tree
between this commits will usually not be well-defined.)

No, every commit should build. This is in part a difference between
Phabricator's patch-based model and GitHub's feature branch model.
However, many projects using the latter also demand that all
intermediate commits must be atomic, buildable changes. Sacrificing this
property greatly complicates bisection.

Building all intermediates is desireable as ultimately we would like to
preserve per-commit build artifacts for the last few months of commits
to enable easy bisection.

I don't quite understand this. Yes building all commits is desirable, but
in the case of Phabricator each revision is going to be a single commit,
no? So why is this an issue? Or is it an issue only for github PRs?

The problem is that many contributors, including Simon PJ, Richard, and
me, tend to push batches of work. For instance, when I land
contributors' differentials I first apply a batch, then validate
locally, and then push as a chunk. We can change this if necessary, but
it will need to be via social convention which hasn't worked very well
historically.

I would like to understand this issue (which prompts #14505) a bit better. Do we agree that, in an ideal world, nobody should ever push to master, and instead, all contributions start as PRs or differentials, which are merged after being reviewed, which crucial includes building them in CI? If that were the case, then we wouldn’t have the issue with needing to force CircleCI to build non-head commits, right?

If we agree that this is the ideal scenario, then what is preventing us from getting to that scenario? In particular, I don’t think you should need to validate contributor code manually on your boxes. The purpose of an automatic CI pipeline is exactly to avoid such manual steps.

I am sorry if any of these questions appear naive.

Cheers,
Manuel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-devops-group/attachments/20171211/031852c6/attachment-0001.html>