today we had an outage caused by whitespace before the leading <?php in a file in wmf-config
there have been different suggestions on how to prevent that in the future:
< ori> we should just check for this very specific thing in scap
< ori> if filename.endswith('.php') and not file.startswith('<?php')
< Krenair> Let's make phpcs voting
< twentyafterfour> at deviantart we had a very strict commit hook that wouldn't accept any file with whitespace before the <?
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | greg | T92531 add a check for whitespace before leading <?php | |||
| Declined | None | T101511 common job for all repositories that runs structural linting | |||
| Declined | None | T111181 use one job for all CI entry points |
See T46875: Jenkins: Create phpcs sniff to detect text before the first <?php tag.
This and various other errors are already caught by phpcs. Please ensure project(s) use phpcs and have it passing/voting.
Straight-out syntax errors are caught by Jenkins too, but we still have the syntax check in scap as a last line of defense. Don't forget that in emergencies, we sometimes have to live-hack code and sync it without waiting on CI.
We can (and should) run composer-test (or phplint/phpcs/phpunit individually) on tin before syncing config changes.
So T92534: scap's check_php_syntax() should check for text before '<?php' in PHP files is about the scap part (with a patch from Lego).
This is only about CI, yes? Bring any scap/deployment related discussion over to T92534 (or a new bug if needed).
I'll clarify the description if that ^^ is indeed true.
Sure. T92534 has already addressed the issue in scap, so let's focus this one on CI.
This follow-up task from an incident report has not been updated recently. If it is no longer valid, please add a comment explaining why. If it is still valid, please prioritize it appropriately relative to your other work. If you have any questions, feel free to ask me (Greg Grossmeier).
I read this bug and I don’t understand what remains to be done [to close this task]:
Are there remaining parts to be checked? perhaps extensions/skins used by WMF?
(This task makes me thinking about another related bug about the closing tag T10493, perhaps it could be added a similar PHPCS rule, but that’s another issue.)