WordPress / twentytwentyone

Ongoing development of Twenty Twenty-One, the new default WordPress theme slated for 5.6.
GNU General Public License v2.0
179 stars 117 forks source link

Code quality: Add Github actions for pull requests #9

Closed carolinan closed 4 years ago

carolinan commented 4 years ago

Each pull request should be checked for PHP compatibility and to ensure that coding standards are followed.

jeffikus commented 4 years ago

Looking into 2 options for this:

  1. https://github.com/marketplace/actions/phpcs-code-review
  2. https://github.com/marketplace/actions/phpcodesniffer

Both perform phpcs checks, but the first option actually logs a review. I'm testing them on a dummy repo and will provide some feedback.

jeffikus commented 4 years ago

The second option is outdated. I tested the first one, and it's half working. I need to:

- [ ] setup a bot user to test. - [ ] verify that it works with the bot user.

youknowriad commented 4 years ago

This runs both phpcs and php unit tests on Gutenberg if you want to reuse. There's a few related things though (wp-env setup, composer...)

https://github.com/WordPress/gutenberg/blob/master/.github/workflows/unit-test.yml#L50-L87

luminuu commented 4 years ago

Does this action may be suitable for this repo? https://github.com/dingo-d/wpthemereview-gh-action

carolinan commented 4 years ago

I don't care if they are actual github actions as long as the pr's are checked so that the theme can be packaged and included into core without errors.

dingo-d commented 4 years ago

Hi! I've added #62 PR that addresses linting issues :) Hope this helps.

carolinan commented 4 years ago

We need to lint the CSS as well, see https://github.com/WordPress-Coding-Standards/stylelint-config-wordpress

aristath commented 4 years ago

We need to lint the CSS as well, see WordPress-Coding-Standards/stylelint-config-wordpress

All scss files were linted and fixed in https://github.com/WordPress/twentytwentyone/pull/96. Next step would be to add auto-linting for them too, but as a first thep #96 fixes all pre-existing issues so we can then add tests

dingo-d commented 4 years ago

We need to lint the CSS as well, see https://github.com/WordPress-Coding-Standards/stylelint-config-wordpress

Maybe that can be a separate PR? Just to get this going? There are linter actions in the GH Actions marketplace so it shouldn't be complicated to set those up 🙂

aristath commented 4 years ago

Maybe that can be a separate PR? Just to get this going? There are linter actions in the GH Actions marketplace so it shouldn't be complicated to set those up slightly_smiling_face

Agreed. The testing part of these will be relatively easy to set-up after we get the initial phpcs checks going :+1:

jeffikus commented 4 years ago

@aristath that test you ran in #60 covers the basics and looks good to go for now. We can add additional libraries as we go - I ran index.php through it as well and it passes if a newline is added at the end of the file.

jeffikus commented 4 years ago

I merged #60 - let's iterate on that based on feedback from PR's and add other linting next.

dingo-d commented 4 years ago

Hi! Did you see #62? This is an improvement on the above-merged feature 🙂

melchoyce commented 4 years ago

What's still needed here?

jeffikus commented 4 years ago

@dingo-d thanks for #62 :) the main reason for merging #60 first was to get something in before the repo went public, but I'm glad your better one is in!

@aristath @dingo-d do we need to do anything more for:

dingo-d commented 4 years ago

There is a stylelint action (CSS), I think we can add ESLint one as well

aristath commented 4 years ago

PHP & CSS are already covered, for JS (eslint) I think we can open a new ticket and get those linted as well. We may not have a lot of JS right now, but we still have a long way to go before the theme is ready and more JS may be added :+1: