Auto-formatting code with black - object now if you have a strong opinion

[Augie Fackler]

On Tue, Nov 27, 2018 at 6:42 PM Gregory Szorc <gregory.szorc at gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 8:13 AM Augie Fackler <raf at durin42.com> wrote:
>>
>>
>>
>> > On Nov 27, 2018, at 11:09, Pulkit Goyal <7895pulkit at gmail.com> wrote:
>> >
>> > I am +1 on using black.
>> >
>> > Adding to what Augie said, we plan to preserve the annotate history by commiting the changes with something like "# skip-blame" in commit message. We can then skip those revisions while annotating using the `--skip` flag of `hg annotate` which accepts a revset, something like `hg annotate --skip "desc('# skip-blame')"`.
>> >
>> > Maybe we should use "# skip-blame black-changes" to mark those changes as black specific and to differentiate between py3 and black changes. (We already use "# skip-blame" for py3 related trivial changes.)

>>
>> Indeed. Note that if we do the black transition, we should probably also do the mass-addition of b prefixes on our string literals at the same time (in a separate commit, obviously) so that people only have painful rebases once and not twice.
>
>
> IMO the annotation should be "# auto-reformat" or something along those lines: you should be annotating the action that was taken, not the side-effects you purport you want to take from it. When skipping revisions for annotate, we can use a revset with all the relevant annotations. e.g. `desc("# auto-reformat") | desc("# py3-compat")`. But we've already started using "# skip-blame" so perhaps this ship has sailed.

auto-reformat seems fine, I'm happy to have both.

>
> Speaking of ships that have sailed, I would prefer we use "annotate" instead of "blame," as the former doesn't have the negative connotations of the latter.
>
> I agree with Augie that a mass-addition of b prefixes should be done at the same time in a separate commit from reformatting.
>
> I have no preferences whether we should have a single massive commit or split things up by file.
>
> I also have some issues with black's formatting. I dislike black's use of double quotes for strings (maybe a holdover from my knowledge of shell, Perl, and PHP, where different quotes matter).

> And I wish it didn't collect imports on the same line.

I think we could work around this by having one import per import statement, eg

from mercurial import hg
from mercurial import ui as uimod

I don't remember why we don't do that today. Is it performance or aesthetics?

> But my objections are insignificant compared to the benefits of having consistently formatted source code. The research in this area shows that the actual style matters less than consistency. If you really care about a style, you can configure your editor to apply your personal source format on file load and the official style format on save. You don't get to do that on remote code review, hgweb, or when looking at diffs from `hg` output. But it's a mitigation if you care that much.