4 ms·
My favourite anti-pattern: Focus all your effort on catching errors that a linter and auto-formatter could catch. Shrug when questioned about architectural dec
by xnorswap 2y ago
My favourite anti-pattern:
Focus all your effort on catching errors that a linter and auto-formatter could catch. Shrug when questioned about architectural decisions, what really matters is that this field was PascalCase when it should be camelCase.
When questioned why this isn't caught automatically by the build process, you have a few stock responses (Bonus: These may well be true!):
- Linters are too slow, and a fast development experience is crucial
- Linting the entire existing code base would effectively destroy "git blame" as a useful tool.
- Different teams can't agree on whether to column-align wrapped function arguments, so they've agreed to disagree on this.
So instead of automating away the mental overhead of checking for spaces vs tabs, or LF vs CRLF make sure you go over the PR with your own linter and nitpick every violation, even the suggestion level ones before you even consider trying to work out the validity of the submission.
- nomel 2y ago> Linters are too slow, and a fast development experience is crucial It's been, literally, 15 years since I've used an editor without linting built in. If someone tells me this, then it just means they don't gaf, and there will be bigger problems with their code.
- sqeaky 2y agoThe most productive team I worked on just ignores this shit in code reviews. Like we will suggest stuff if the obvious formatting is wrong in a way the author probably didn't intend, but we're not going to hold up a code review on a nit. If there's one variable that's Pascal case instead of camel case or something that's not going to stop the code from going in if it's good code from algorithmic and documentation perspective. We just vaguely try to be consistent in each file and act like adults. We really agreed on value. We would insist that code that could be tested had tests. It was a C++ code base so we insisted that things be as fast as reasonable suggesting proper algorithms and data access patterns. And in general actually cared about bugs and making then shallow. Another point of agreement was that an automatic code formatter almost always made it look worse than having a few nits here and there. Anyone who mandates the things that really don't matter is just getting in the way.
- kolme 2y agoI think it's kind of important to be consistent about case because if not, people will trip when typing, it's annoying and might even cause bugs. One thing I agree though, it's a waste of time to discuss about it. That problem is best solved with automatic tooling / linting. In the editor, git hooks and CI. But if you and your colleagues don't have a problem with it, then I guess it's fine.
- xnorswap 2y agoIf you don't have an automatic formatter, then you'll find you get messy diffs when it's worked on by someone who does have an automatic formatter. Worse still, you'll find you get messy diffs in constant ping-pong because different people have their formatters configured in different ways. This is a solved problem, many editors, and tools, respect .editorconfig. Learn to configure it, and never have to think about formatting again.
- sqeaky 2y agoIf someone's running an automatic formatter on someone else's project that doesn't match that will always cause a problem.
- kolme 2y agoPossible solution: mandatory pre-commit or pre-push hook that lints the code.
- Marsymars 2y agoWe have this, but have a handful of devs who can't be bothered to install the hooks locally so deal with their PRs by fixing one linter error at a time and pushing a new commit to see what fails in the CI.
- move-on-by 2y agoI’m glad I’m not the only one who has to put up with fools like this. Not only the insane git log, but the compute for those actions isn’t free either. Just run the damn precommit hooks, it literally fixes the issues for you!
- fragmede 2y agoyou do squash though, right? ...right?
- rurban 2y agoTheir fault then. It's their time afterall. It cannot be much easier than to install pre-commit hooks.
- TeeMassive 2y ago> Linting the entire existing code base would effectively destroy "git blame" as a useful tool. Put a '.git-blame-ignore-revs' file at the root of your project with commit hashes (and # comments). GitHub and other popular git tools will consider it.
- xnorswap 2y agoThat's a top tip, thanks, I had no idea about that feature.
- move-on-by 2y agoVery interesting! It seems this is a local-only solution using `git config`.