Previously I had shied away from ideas similar to (a) or (b) since I wasn't sure if that would be what developers would expect and accept, given that it would cause the change the reviewer sees to no longer match their local change, which could cause hicups e.g. when trying to find the code that a reviewer has commented on locally, or when rebasing on top of a newer version of mozilla-central after Lando has landed your patch (which may cause conflicts with your local patch due to formatting differences).
Do we think these types of issues aren't something we should be concerned about? Thanks, Ehsan On Fri, Dec 7, 2018 at 3:09 PM Andrew Halberstadt <a...@mozilla.com> wrote: > I think we should implement a) and do the formatting prior to submission. > This prevents us from wasting reviewer time on format issues, and also > makes sure that "what you see in phab, is what gets landed". > > On Fri, Dec 7, 2018, 2:04 PM Gregory Szorc, <g...@mozilla.com> wrote: > >> On Fri, Dec 7, 2018 at 1:57 PM Botond Ballo <bba...@mozilla.com> wrote: >> >> > On Fri, Dec 7, 2018 at 11:36 AM Sylvestre Ledru <sle...@mozilla.com> >> > wrote: >> > > In the meantime, we will be running a bot weekly to reformat the >> > > mistakes and add the changeset into the ignore lists. >> > > But in the long run this won’t be sustainable, so once we gain >> > > confidence that a good number of developers have successfully >> integrated >> > > clang-format into their local workflow, we will look into enabling a >> > > Mercurial hook on hg.mozilla.org to reject misformatted code upon >> push >> > > time. That will be the ultimate solution to help ensure that our code >> > > will be properly formatted at all times. >> > >> > Have you considered something like running clang-format automatically >> > during landing (i.e. as part of what Lando does to the patch)? That >> > seems like it would achieve the best of both worlds, by not placing >> > any requirements on what developers do locally, while also ensuring >> > the tree is always properly formatted without cleanup commits. >> > >> >> I chatted with Sylvestre earlier today. While I don't want to speak for >> him, I believe we both generally agree that the formatting should happen >> "automagically" as part of the patch review and landing lifecycle, even if >> the client doesn't have their machine configured for formatting on save. >> This would mean that patches are either: >> >> a) auto-formatted on clients as part of being submitted to Phabricator >> b) updated automatically by bots after submission to Phabricator >> c) auto-formatted by Lando as part of landing >> >> Lando rewrites/rebases commits as part of landing, so commit hashes >> already >> change. So if auto-formatting magically occurs and "just works" as part of >> the review/landing process, there should be little to no developer >> inconvenience compared to what happens today. i.e. developers don't need >> to >> do anything special to have their patches land with proper formatting. >> _______________________________________________ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform >> > -- Ehsan _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
