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

I'm a little stricter.

In terms of formatting etc I rely on tooling which break the CI. That way they don't even surface. Equally if something slips through the CI needs to be updated.

A piece of code should match the general style of the code it is being integrated into. When you create an addition to a wood-framed house the addition won't be steel construction, unless you have an explicit reason (weight, speed, foundation etc). And if I'm the main maintainer of a codebase, it is something I can call out on. If we use Tokio and you use raw threads I'll ask about this choice.

In an ideal world you (the PR author) includes this in its description, because it is a reflection of your understanding of the codebase.

Reviewing code with 'will this work' is also debatable. Someone might propose a change that fixes an issue they are having. But maybe it's not something we want to solve in our codebase, or maybe it breaks other things the maintainers are thinking off.



> In terms of formatting etc I rely on tooling which break the CI. That way they don't even surface. Equally if something slips through the CI needs to be updated

I also do something similar with lint rules. For Rust, this means turning on every single reasonable clippy lint (there are many), and making all warnings into errors. Some would say that some rules are overzealous or annoying, but it's a price I'm willing to pay to keep the codebase uniform.


Same here, I have a Cargo.toml in which I painstakingly went through every lint and decided whether it made sense to me.


I contributed to an OSS project recently that ran the linter before the tests and it absolutely trashed my code-build-test cycle. If I felt less strongly about the value of the project I would have bailed.

If I’m trying to get the code to work don’t cockblock me on Make it Work with your Make it Right enforcements.


That is a bit overzealous. It makes sense for CI where you don't want to waste running tests on an already failing build, but for local builds you probably want to allow warnings and be able to run tests, lints, and formatting checks separately.


I will probably file a PR to flip them around. Since the runscript will stop on a broken test it’ll work better for local.

And then there’s the duplication and differences between the runscripts and the GitHub actions…


I’ve heard this argument before but I don’t get it. It’s trivial to setup your code editor to format on save. Why do people not do that but rather complain about files linter checks on CI?


I don't think they were talking about formatting issues, since these tend to not change the behaviour of the code (outside of Python) so that you can't use the tests before addressing the linter issues.


Biome can fail the lint process if there’s an extra new line. Or you shortened a line that it thinks shouldn’t be wrapped anymore, or you added an argument to a line that it thinks should. I hates it.


It’s trivial to set up my code editor to save to the format of six different GitHub repos? Do tell.

One of them uses tabs. Fucking Richard.


Your code editor should respect the .editorconfig of the repo you are editing, so yeah, trivial.


Bold of you to assume aggravating projects have an editorconfig. (Checked, the worst doesn't)


Only one does as it turns out.




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

Search: