Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

At Facebook we only tell you about lint violations for the lines you touch using arcanist from phabricator[1]. While it works great for most lint warnings, this hasn't worked that well for code formatters.

The most successful strategy was to add a flag in the file (@format in the header) to tell that a file is automatically formatted. The immediate benefit is that we enable format on save for developers on those files when they use Nuclide (>90% of penetration for JavaScript and Hack).

The other advantage is that when we release a new version of the formatter, we can re-run it on all those files so that people don't have lint warnings on code they already formatted in the past.

With that setup, there's a strong incentive for individual engineers to run the formatter on their team codebase in one PR and then everyone benefits from now on.

[1] https://secure.phabricator.com/



Have you run into issues where the "let's reformat the entire codebase" commit makes `git blame` unusable?


It doesn't. Use `git hyper-blame` or `git blame $REV^ -- $PATH`.

Sure, there is an additional step but we feel this shouldn't be a blocker for significant workflow improvements.

In fact, a single big "reformat all" commit is better than a bunch of incremental ones that reformat areas that you also change semantically. That is harder to filter and makes diffs harder to follow (which changes are logic and which are just style?).


I hadn't head of hyper-blame before. It's part of chromium's depot_tools.

> git hyper-blame is like git blame but it can ignore or "look through" a given set of commits, to find the real culprit.

https://commondatastorage.googleapis.com/chrome-infra-docs/f...




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: