
<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="">Am 14.12.2017 um 01:36 schrieb Simon Marlow <<a href="mailto:marlowsd@gmail.com" class="">marlowsd@gmail.com</a>>:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="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;" class=""><div class="gmail_extra"><div class="gmail_quote">On 12 December 2017 at 08:12, Manuel M T Chakravarty<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:manuel.chakravarty@tweag.io" target="_blank" class="">manuel.chakravarty@tweag.io</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class="">Hi Ben,</div><span class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">Am 12.12.2017 um 02:22 schrieb Ben Gamari <<a href="mailto:ben@well-typed.com" target="_blank" class="">ben@well-typed.com</a>>:</div><br class="m_5787158424590649828Apple-interchange-newline"><div class=""><div class="">Simon Peyton Jones <<a href="mailto:simonpj@microsoft.com" target="_blank" class="">simonpj@microsoft.com</a>> writes:<br class=""><br class=""><blockquote type="cite" class="">The problem is that many contributors, including Simon PJ, Richard, and<br class="">me, tend to push batches of work<br class=""><br class="">I have not been following this thread (“job accounting” seemed above<br class="">my pay grade) but I saw this mention of my name 😊. Without having<br class="">read myself into the context there seem to be two issues<br class=""><br class=""><br class=""> * Every commit to master should be validate-clean, and this should<br class=""> be tested by the CI framework not by the contributor. This is<br class=""> essential. I would be delighted if every commit I made went through<br class=""> that gate. I’m careful, but occasionally not careful enough.<br class=""><br class=""> * Most – perhaps all – commits should go through a code-review<br class=""> process. Here I freely admit that I tend to use (or abuse?) my<br class=""> status to make most of my commits without review, except perhaps<br class=""> informally with individuals. I’d be absolutely willing to review<br class=""> this if (a) in fact people think that the extra step would really<br class=""> improve quality (perhaps looking at past commits) or (b) the very<br class=""> fact that I do so makes people feel cross.<br class=""><br class=""></blockquote>I personally think that we should strive for your first point (every<br class="">commit should be validate-clean) before attempting to tackle your<br class="">second. I, for one, am rather skeptical that putting all of your patches<br class="">through review would significantly affect quality.<br class=""></div></div></blockquote><div class=""><br class=""></div></div></span>I completely agree.<div class=""><br class=""></div><div class="">So, what is preventing us from disabling direct pushes to master and requiring all contributions to go through a PR or Differential? </div></div></blockquote><div class=""><br class=""></div><div class="">Well, CI needs to be working first :)</div></div></div></div></div></blockquote><div><br class=""></div><div>CI itself is working for Linux and macOS, and the hold up with Windows is largely us trying to get it for free from AppVeyor. The outstanding problems are with getting Phabricator to integrate/cooperate and now agreeing on a workflow. If we would use GitHub and PRs (like most of the rest of the world), I think, all this would be solved already also. </div><div><br class=""></div><div>Custom infrastructure => extra costs, as usual.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" style="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;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">Also Phabricator doesn't have the equivalent of a merge button right now, which makes the workflow a bit awkward. I'm not sure what the current state of that is - is there an extension or something we can enable to get this, Ben?<br class=""></div><div class=""><br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="word-wrap: break-word; line-break: after-white-space;" class=""><div class="">PRs and Differentials are squashed on merging to master and the whole problem (with CircleCI building only heads of commit groups) just disappears. I believe that this is the usual approach.</div></div></blockquote><div class=""><br class=""></div><div class="">How do we enforce that PRs get squashed on merging? (not that we’re actively using PRs (yet) but I’m curious how this works).<br class=""></div></div></div></div></div></blockquote><div><br class=""></div>GitHub has a per-repository setting allowing a choice between three options as follows.</div><div><br class=""></div><div><span style="color: rgb(36, 41, 46); font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol"; font-size: 14px;" class="">When merging pull requests, you can allow any combination of merge commits, squashing, or rebasing. At least one option must be enabled.</span></div><div><div class="Box" style="box-sizing: border-box; background-color: rgb(255, 255, 255); border: 1px solid rgb(209, 213, 218); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; color: rgb(36, 41, 46); font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol"; font-size: 14px;"><form accept-charset="UTF-8" action="https://github.com/mchakravarty/HaskellForMac/settings/update_merge_settings" class="repository-merge-features js-merge-features-form" data-autosubmit="true" data-remote="true" method="post" style="box-sizing: border-box;"><div style="box-sizing: border-box; margin: 0px; padding: 0px; display: inline;" class=""></div><ul style="box-sizing: border-box; padding-left: 0px; margin-top: 0px; margin-bottom: 0px;" class=""><li class="Box-row py-0" style="box-sizing: border-box; padding-top: 0px !important; padding-right: 16px; padding-bottom: 0px !important; padding-left: 16px; margin-top: -1px; list-style-type: none; border-top-width: 1px; border-top-style: solid; border-top-color: transparent; border-top-left-radius: 2px; border-top-right-radius: 2px;"><div class="js-repo-option form-group" style="box-sizing: border-box; margin: 15px 0px;"><div class="form-checkbox" style="box-sizing: border-box; padding-left: 20px; margin: 15px 0px; vertical-align: middle;"><label for="merge_types_merge_commit" style="box-sizing: border-box; font-weight: 600; position: static;" class="">Allow merge commits</label> <span class="status-indicator js-status-indicator" style="box-sizing: border-box; display: inline-block; width: 16px; height: 16px; margin-left: 5px;"></span><input type="checkbox" name="merge_types[]" value="merge_commit" id="merge_types_merge_commit" checked="" style="font-family: inherit; font-size: inherit; font-style: inherit; font-variant-caps: inherit; font-stretch: inherit; line-height: inherit; margin: 5px 0px 0px -20px; overflow: visible; padding: 0px; float: left; vertical-align: middle;" class=""><div style="box-sizing: border-box; margin: 0px; min-height: 17px; font-size: 12px; color: rgb(88, 96, 105);" class="">Add all commits from the head branch to the base branch with a merge commit.</div></div></div></li><li class="Box-row py-0" style="box-sizing: border-box; padding-top: 0px !important; padding-right: 16px; padding-bottom: 0px !important; padding-left: 16px; margin-top: -1px; list-style-type: none; border-top-width: 1px; border-top-style: solid; border-top-color: rgb(225, 228, 232);"><div class="js-repo-option form-group" style="box-sizing: border-box; margin: 15px 0px;"><div class="form-checkbox" style="box-sizing: border-box; padding-left: 20px; margin: 15px 0px; vertical-align: middle;"><label for="merge_types_squash" style="box-sizing: border-box; font-weight: 600; position: static;" class="">Allow squash merging</label> <span class="status-indicator js-status-indicator" style="box-sizing: border-box; display: inline-block; width: 16px; height: 16px; margin-left: 5px;"></span><input type="checkbox" name="merge_types[]" value="squash_merge" id="merge_types_squash" checked="" style="font-family: inherit; font-size: inherit; font-style: inherit; font-variant-caps: inherit; font-stretch: inherit; line-height: inherit; margin: 5px 0px 0px -20px; overflow: visible; padding: 0px; float: left; vertical-align: middle;" class=""><div style="box-sizing: border-box; margin: 0px; min-height: 17px; font-size: 12px; color: rgb(88, 96, 105);" class="">Combine all commits from the head branch into a single commit in the base branch.</div></div></div></li><li class="Box-row py-0" style="box-sizing: border-box; padding-top: 0px !important; padding-right: 16px; padding-bottom: 0px !important; padding-left: 16px; margin-top: -1px; list-style-type: none; border-top-width: 1px; border-top-style: solid; border-top-color: rgb(225, 228, 232); border-bottom-right-radius: 2px; border-bottom-left-radius: 2px;"><div class="js-repo-option form-group" style="box-sizing: border-box; margin: 15px 0px;"><div class="form-checkbox" style="box-sizing: border-box; padding-left: 20px; margin: 15px 0px; vertical-align: middle;"><label for="merge_types_rebase" style="box-sizing: border-box; font-weight: 600; position: static;" class="">Allow rebase merging</label> <span class="status-indicator js-status-indicator" style="box-sizing: border-box; display: inline-block; width: 16px; height: 16px; margin-left: 5px;"></span><input type="checkbox" name="merge_types[]" value="rebase_merge" id="merge_types_rebase" checked="" style="font-family: inherit; font-size: inherit; font-style: inherit; font-variant-caps: inherit; font-stretch: inherit; line-height: inherit; margin: 5px 0px 0px -20px; overflow: visible; padding: 0px; float: left; vertical-align: middle;" class=""><div style="box-sizing: border-box; margin: 0px; min-height: 17px; font-size: 12px; color: rgb(88, 96, 105);" class="">Add all commits from the head branch onto the base branch individually.</div></div></div></li></ul></form></div><div><br class=""></div><div><br class=""></div><div>If more than one option is allowed, you can choose which one to use at the time of pressing the merge button.</div><div><br class=""></div><div>Cheers,</div><div>Manuel</div><div><br class=""></div></div></body></html>