If your coding standards takes 3 minutes to run in CI, then you’ve messed up your build configuration.
We run php lint, cs, and phpstan in under 2 minutes on a big project and in parallel to tests. It doesn’t matter how long it takes, as long as the tests need to run anyway. So, yeah I think it comes down to how you setup your build.
Not really. Including CI booting time, blocking other parallel jobs, Docker image etc.
Coding Standard Rules must be Fixable
Sure, in a perfect world but there are plenty of things that require a human to fix safely. Following naming conventions in general, be they file name, class name or variable name requirements.
There are also plenty of things that require human knowledge not inferable from the code itself. Off the top of my head visibility, and missing docs are good examples. For instance we require all untyped parameters to have an explanatory usage comment, and have a custom sniff to enforce it.
I agree with the general premise that everything that can be automatically fixed should be, but disagree that everything that can be considered part of a coding standard can be automatically fixed. I also think in some cases there is value in the developers being told not to do things as a learning experience.
I mention this mixup in the post. We have so many tools in PHP, that it’s unclear what should do what. We have coding standard tools handling upgrade and AST tools handling extra comma. It’s a mess, because AST and tokens overlap each other.
If coding standard give you 2 and more options, it should be definitelly handled by human. No argument here. That’s not in scope of coding standard, but rather static analysis guide.
I didn’t get how it’s suggested to enforce the coding standards so it doesn’t get on tree.
I’m not sure I understand. Could you elaborate a bit more?
I feel this article is missing a key point, only hinting at why we even bother with coding standards in the first place.
Our PHP code should look the same.
Why do we want to do this? For me and our development team is because it makes it easier to review changes and also to reduce cosmetic code changes. Where one can argue that the second one is to facilitate the first one.
So yes, I am in favor of having a CI check the code style because you do not have to worry about and it makes changes minimal without all the visual clutter so you can focus on the actual change.
We’re on the same boat here. You can achieve the same goal without actually having coding standard in CI on every pull-request.
… Or you could just make a githook that formats and then tests your code before you commit or push.
Having said that, if you have code that could break when formatted, you should be asking entirely different questions
If the blocker is in CI or locally, it still a wasted waiting time. The goal is to have 0 seconds spent on waiting to run coding standard tool.
Where’s the answer? I didn’t get the point of the article.
I just let php cs fixer auto commit in CI and the whole team is happy and 0 time is spent thinking about this in code review. And developers time is more expensive then CI minute time.