<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div><blockquote type="cite" class=""><div class="">Simon Peyton Jones <<a href="mailto:simonpj@microsoft.com" class="">simonpj@microsoft.com</a>>:</div><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 16px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0cm 0cm 0.0001pt 36pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class="">The problem is that many contributors, including Simon PJ, Richard, and<br class="">me, tend to push batches of work<o:p class=""></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class="">I have not been following this thread (“job accounting” seemed above my pay grade) but I saw this mention of my name<span class="Apple-converted-space"> </span></span><span style="font-size: 12pt; font-family: "Segoe UI Emoji", sans-serif;" class="">😊</span><span style="font-size: 12pt;" class="">. Without having read myself into the context there seem to be two issues<o:p class=""></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><ul type="disc" style="margin-bottom: 0cm; margin-top: 0cm;" class=""><li class="MsoListParagraph" style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><span style="font-size: 12pt;" class="">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.<br class=""><br class=""><o:p class=""></o:p></span></li><li class="MsoListParagraph" style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><span style="font-size: 12pt;" class="">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. <span class="Apple-converted-space"> </span></span></li></ul></div></div></blockquote><div><br class=""></div><div>I think, the important point, especially in the context we are discussing here is the first one. We should ensure that every single commit goes through CI before hitting master.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 16px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class="">In both cases, the big thing from my point of view is that, once I’m ready to press “go”,<span class="Apple-converted-space"> </span><i class="">I’d like to take it off my to-do list</i><span class="Apple-converted-space"> </span>by pushing into a queue that will result in either<o:p class=""></o:p></span></div><ul type="disc" style="margin-bottom: 0cm; margin-top: 0cm;" class=""><li class="MsoListParagraph" style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><span style="font-size: 12pt;" class="">a commit to master, or<o:p class=""></o:p></span></li><li class="MsoListParagraph" style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><span style="font-size: 12pt;" class="">an email to me saying “more work to do here”<o:p class=""></o:p></span></li></ul><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class="">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.</span></div></div></div></blockquote><div><br class=""></div><div>The idea with pull requests on GitHub is basically what you are describing. CI automatically validates the pull request and a reviewer can just push the merge button if CI is green and they are happy. If they are unhappy, the comment, which results in a notification to you.</div><div><br class=""></div><div>I always thought this is what Differentials in Phabricator are for. </div><div><br class=""></div><div>Ben, is that not so?</div><div><br class=""></div><blockquote type="cite" class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-size: 11pt; margin: 0cm 0cm 0.0001pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class="">Also important: I often accumulate a sequence of related patches:<o:p class=""></o:p></span></div><div style="font-size: 11pt; margin: 0cm 0cm 0.0001pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><ul type="disc" style="font-size: 16px; margin-bottom: 0cm; margin-top: 0cm;" class=""><li class="MsoListParagraph" style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><span style="font-size: 12pt;" class="">I’d like to be able to queue them as a group, or at least with very low additional overhead per patch<o:p class=""></o:p></span></li></ul><div style="font-size: 11pt; margin: 0cm 0cm 0.0001pt 36pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><ul type="disc" style="font-size: 16px; margin-bottom: 0cm; margin-top: 0cm;" class=""><li class="MsoListParagraph" style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;"><span style="font-size: 12pt;" class="">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. <o:p class=""></o:p></span></li></ul><div style="font-size: 11pt; margin: 0cm 0cm 0.0001pt 36pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><div style="font-size: 11pt; margin: 0cm 0cm 0.0001pt 36pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class="">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.</span></div></div></blockquote><div><br class=""></div>This is where things get a bit tricky.</div><div><br class=""></div><div>A standard solution is to ”squash” commits; i.e., these separate patches are combined into one when they are finally applied to master. This means that the separate patches are never tested separately and that ”broken” intermediate state is never observed by CI or bisection. (The patches are still separate during code review.)<br class=""><div><br class=""></div><div>Manuel</div><br class=""><blockquote type="cite" class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 16px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class="">I’m very keen to support a better CI process in any way I can.<o:p class=""></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class="">Simon<o:p class=""></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 12pt;" class=""><o:p class=""> </o:p></span></div><div style="border-style: none none none solid; border-left-width: 1.5pt; border-left-color: blue; padding: 0cm 0cm 0cm 4pt;" class=""><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0cm 0cm;" class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><b class=""><span lang="EN-US" class="">From:</span></b><span lang="EN-US" class=""><span class="Apple-converted-space"> </span>Ghc-devops-group [<a href="mailto:ghc-devops-group-bounces@haskell.org" class="">mailto:ghc-devops-group-bounces@haskell.org</a>]<span class="Apple-converted-space"> </span><b class="">On Behalf Of<span class="Apple-converted-space"> </span></b>Manuel M T Chakravarty<br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>11 December 2017 06:17<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Ben Gamari <<a href="mailto:ben@well-typed.com" class="">ben@well-typed.com</a>><br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:ghc-devops-group@haskell.org" class="">ghc-devops-group@haskell.org</a>; Mateusz Kowalczyk <<a href="mailto:fuuzetsu@fuuzetsu.co.uk" class="">fuuzetsu@fuuzetsu.co.uk</a>><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [GHC DevOps Group] CircleCI job accounting question<o:p class=""></o:p></span></div></div></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">23.11.2017 06:15 schrieb Ben Gamari <<a href="mailto:ben@well-typed.com" style="color: purple; text-decoration: underline;" class="">ben@well-typed.com</a>>:<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class="">Simon Marlow <</span><a href="mailto:marlowsd@gmail.com" style="color: purple; text-decoration: underline;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class="">marlowsd@gmail.com</span></a><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class="">> writes:<br class=""><br style="font-variant-caps: normal; text-align: start; -webkit-text-stroke-width: 0px; word-spacing: 0px;" class=""><br class=""></span><o:p class=""></o:p></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class="">On 22 November 2017 at 15:31, Ben Gamari <<a href="mailto:ben@well-typed.com" style="color: purple; text-decoration: underline;" class="">ben@well-typed.com</a>> wrote:<br class=""><br class=""><br class=""><o:p class=""></o:p></span></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class="">Manuel M T Chakravarty <<a href="mailto:manuel.chakravarty@tweag.io" style="color: purple; text-decoration: underline;" class="">manuel.chakravarty@tweag.io</a>> writes:<br class=""><br class=""><o:p class=""></o:p></span></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class="">Why do we need the intermediate builds exactly? Wouldn’t they usually<br class="">fail? (When I do PRs with multiple commits, the state of the tree<br class="">between this commits will usually not be well-defined.)<o:p class=""></o:p></span></div></blockquote><p class="MsoNormal" style="margin: 0cm 0cm 12pt; font-size: 11pt; font-family: Calibri, sans-serif;"><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class=""><br class="">No, every commit should build. This is in part a difference between<br class="">Phabricator's patch-based model and GitHub's feature branch model.<br class="">However, many projects using the latter also demand that all<br class="">intermediate commits must be atomic, buildable changes. Sacrificing this<br class="">property greatly complicates bisection.<br class=""><br class="">Building all intermediates is desireable as ultimately we would like to<br class="">preserve per-commit build artifacts for the last few months of commits<br class="">to enable easy bisection.<o:p class=""></o:p></span></p></blockquote><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class=""><br class="">I don't quite understand this. Yes building all commits is desirable, but<br class="">in the case of Phabricator each revision is going to be a single commit,<br class="">no? So why is this an issue? Or is it an issue only for github PRs?<o:p class=""></o:p></span></div></blockquote><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span style="font-size: 11.5pt; font-family: Menlo-Regular, serif;" class=""><br class="">The problem is that many contributors, including Simon PJ, Richard, and<br class="">me, tend to push batches of work. For instance, when I land<br class="">contributors' differentials I first apply a batch, then validate<br class="">locally, and then push as a chunk. We can change this if necessary, but<br class="">it will need to be via social convention which hasn't worked very well<br class="">historically.</span><o:p class=""></o:p></div></div></blockquote><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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?<o:p class=""></o:p></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">I am sorry if any of these questions appear naive.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Cheers,<o:p class=""></o:p></div></div><div class=""><div style="margin: 0cm 0cm 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Manuel</div></div></div></div></blockquote></div><br class=""></body></html>