This definitely looks like the wrong brace style to apply and defeats the point of my suggestion to add the braces before applying Prettier as this would change the blame for the body of the if rather than preserve it.

You can probably autofix the [[ https://eslint.org/docs/rules/brace-style | brace-style ]] on these files in the same commit.

This is what ESLint does for autofix.

The whole purpose of doing this is being able to land things incrementally (albeit all in a single day). Adding the brace rule to recommended.js means that landing incrementally is impossible, because the ESlint task would fail.

Can you please why tagging these changesets for blame skip wouldn't work?

"[...] please *explain* why", there's a typo there :)

See also https://lists.mozilla.org/pipermail/dev-platform/2019-January/023284.html and https://searchfox.org/mozilla-central/source/.git-blame-ignore-revs

I wasn't suggesting to enable the brace-style rule permanently, only during your autofix for curly so that it doesn't introduce this intermediate unwanted state and unnecessarily change the blame of the body of the if. If you don't want to autofix the brace-style for your newly introduced incorrect braces then I would prefer if you added the braces in the same commit where Prettier runs to not have the intermediate bad state.

Blame skipping isn't an excuse to change blame unnecessarily since not all tools even know about the concept. I already explained how blame skipping can potentially have problems when you're folding the if and the body into one line and then back to two lines.

If you fold the curly changes into the commits that format with Prettier then I think we're good. Otherwise, please fix the formatting of this intermediate revision to have proper brace style and not change the blame on the if body lines.

Just wanted to clarify something: this is what the "curly" auto fix does; this is eslint's interpretation of how this rule should be satisfied. These aren't manually introduced braces.

Folding these changes as you've suggested is possible, but makes the landing much more difficult, which is something I wanted to avoid. At the very least, it introduces a delicate dance between satisfying ESLint and landing prettification changesets incrementally (one top-level directory at a time).

If you feel so strongly about this to block landing, I'll fold these changesets with the Prettier ones.

Alright, so after rereading this I think what would make sense is introduce (or keep) "brace-style" in recommended.js when turning "curly: all" everywhere, then having a followup commit where "brace-style" is removed. This would end up regenerating all of the auto-fix changesets, assuming that eslint works properly.

Victor and I looked at this, and unfortunately eslint with brace-style + curly set gets the indentation wrong. We'd need to fix the indentation as well...

So, what we've agreed on is to keep curly on in recommended.js in this changeset, but have an override in the top-level .eslintrc.js that turns it off for the individual directories. When each individual directory is formatted, the exclusion will also be removed.

This will effectively put the curly change and prettier formatting in the one changeset.