Skip to content

Conversation

@StevenDufresne
Copy link

@StevenDufresne StevenDufresne commented Dec 9, 2024

This PR changes the linting action to only run when phpcs is present so we can use it with the parent theme which doesn't include any PHP. That PR exists here: WordPress/wporg-parent-2021#166.

@ryelle
Copy link
Contributor

ryelle commented Dec 9, 2024

Is this to prevent the error Command "lint:php" not found. in https://github.com/WordPress/wporg-parent-2021/actions/runs/12245433887/job/34159295938 ?

Some repos won't have phpcs.xml but should still run the linter, because the default file is phpcs.xml.dist, which is installed by the setup tools command.

What I thought we'd do, if there were projects that didn't need to run specific lints, was just define empty commands in the project. So I'd update wporg-parent to say "lint:php": "echo \"No PHP lint.\"" It appears to already do this for the lint:js command.

@StevenDufresne
Copy link
Author

StevenDufresne commented Dec 9, 2024

Some repos won't have phpcs.xml but should still run the linter, because the default file is phpcs.xml.dist, which is installed by the setup tools command.

We could check for both. It don't mind either approach.

@ryelle
Copy link
Contributor

ryelle commented Dec 9, 2024

We could check for both. It don't mind either approach.

I think all projects should install the repo tools, so they'll all have phpcs.xml.dist. It's just a matter of whether they've defined the yarn command that's the issue, right?

@StevenDufresne
Copy link
Author

I think all projects should install the repo tools, so they'll all have phpcs.xml.dist. It's just a matter of whether they've defined the yarn command that's the issue, right?

True, but shouldn't they all have the yarn command defined if they include phpcs.xml.dist? I think wporg-parent is the outlier in this case. I'm fine with adding a noop to parent as well.

@ryelle
Copy link
Contributor

ryelle commented Dec 11, 2024

True, but shouldn't they all have the yarn command defined if they include phpcs.xml.dist? I think wporg-parent is the outlier in this case.

Probably, but it's up to each person who set up the repos to make sure those exist. Having this error out with command not found seems correct in that case, as a prompt to add the lint:php command (even if it's an outlier noop command).

@StevenDufresne
Copy link
Author

Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants