Page MenuHomePhabricator

sync-* and scap should only lint changed files from last deploy
Open, LowPublic

Event Timeline

Reedy raised the priority of this task from to Needs Triage.
Reedy updated the task description. (Show Details)
Reedy added subscribers: Aklapper, Reedy, StudiesWorld.

Change 265273 had a related patch set uploaded (by Reedy):
<?PHP -> <?php due to scap bug

https://gerrit.wikimedia.org/r/265273

Change 265273 merged by jenkins-bot:
<?PHP -> <?php due to scap bug

https://gerrit.wikimedia.org/r/265273

Change 265274 had a related patch set uploaded (by Reedy):
<?PHP -> <?php due to scap bug

https://gerrit.wikimedia.org/r/265274

Change 265274 merged by jenkins-bot:
<?PHP -> <?php due to scap bug

https://gerrit.wikimedia.org/r/265274

Note, these patches don't fix the issue in this bug with scap vs sync-dir behaviour. Just stops <?PHP breaking stuff when it's valid PHP etc

scap seemingly allows <?PHP, but sync-dir doesn't?

scap only checks the files in the config directory for sanity and assumes that Jenkins checked other things so that's probably the difference there.

Part of me thinks scap should do it too... But it's extra time. In the grand scheme of how long scap takes now, it's probably not a great deal

But, I guess, fixing the wrong check in this case, is enough to fix this; as both <?PHP and <?php are valid, just our implemented checker was a bit naive (even if <?PHP isn't common)

Having scap lint every php file in the active branches is going to be slow and I honestly don't see the value in it. sync-file/sync-dir lint is more reasonable as they may be linting security patches or config files that haven't been through a linter in the CI tool chain.

We should generate a list of changed files and only lint those.

greg renamed this task from sync-dir doesn't like <?PHP, but scap does? to sync-* and scap should only lint changed files from last deploy.Apr 4 2016, 9:01 PM
greg triaged this task as Low priority.
greg updated the task description. (Show Details)