WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
207 stars 38 forks source link

Trying to utilise GH actions for running checks instead of travis #261

Closed dingo-d closed 3 years ago

dingo-d commented 3 years ago

This is an attempt to run actions instead of travis and see if we can acchieve everything using them.

The travis stages will be split into 2 workflow files:

I'll see if this works and if we'll achieve what we have on travis currently. Plus we can see which one is faster if we manage to do this 😄

EDIT

How to run this locally?

If anybody is interested in running the workflows locally, you'll need to install act.

You can run the stages individually with

act -j qa_checks -P ubuntu-latest=shivammathur/node:latest -s GITHUB_TOKEN=
act -j tests -P ubuntu-latest=shivammathur/node:latest -s GITHUB_TOKEN=

You'll need to add the GH token so that you avoid GitHub's rate limit for composer.

dingo-d commented 3 years ago

Thanks for the suggestions @desrosj I really appreciate it. I'll refactor this so that it looks a lot more like your WPCS PR, with allowed failures and cleaner syntax.

desrosj commented 3 years ago

Any time! Great work here, especially in a first pass! I'll be a bit absent until the new year, but I'll check in when I notice changes as I'm able.

dingo-d commented 3 years ago

I've fixed the comments from the PR, and also made some additional changes so that it works on composer v2 (dealerdirect had to be raised to 0.7).

Also, fixed some readme, contributing, and other minor issues (I could cherry-pick them to a separate PR to keep this one cleaner 🤔 ).

@jrfnl if you have the time I'd love if you could check this out. It's a lot smaller and I recon easier to check than the WPCS one 😄

Potherca commented 3 years ago

@dingo-d I'm thinking of creating an action (as part of @pipeline-components) for combining these:

            - name: Install xmllint
              run: sudo apt-get install --no-install-recommends -y libxml2-utils

            # Show XML violations inline in the file diff.
            # @link https://github.com/marketplace/actions/xmllint-problem-matcher
            - uses: korelstar/xmllint-problem-matcher@v1

            - name: Validate ruleset against schema
              run: xmllint --noout --schema ./vendor/squizlabs/php_codesniffer/phpcs.xsd ./*/ruleset.xml

into this:

            - name: xmllint
              uses: pipeline-componets/xmllint@v1
              run: xmllint --noout --schema ./vendor/squizlabs/php_codesniffer/phpcs.xsd ./*/ruleset.xml

Is that something you would be interested in?

//CC: @mjrider

jrfnl commented 3 years ago

@Potherca I like it :+1: One question: how would this work if you need to run multiple xmllint commands ? Would they all need to be in the same step ? Would you need to use the action multiple times ?

Example: https://github.com/PHPCompatibility/PHPCompatibilityParagonie/blob/7a71578674453fc293bd8e7b201347a0663b6d16/.github/workflows/ci.yml#L35-L51

Potherca commented 3 years ago

Either should work:

Run multiple commands in one step

      - uses: pipeline-componets/xmllint@v1      
        run: |
          xmllint --noout --schema vendor/squizlabs/php_codesniffer/phpcs.xsd ./*/ruleset.xml
          diff -B ./PHPCompatibilityParagonieRandomCompat/ruleset.xml <(xmllint --format "./PHPCompatibilityParagonieRandomCompat/ruleset.xml")
          diff -B ./PHPCompatibilityParagonieSodiumCompat/ruleset.xml <(xmllint --format "./PHPCompatibilityParagonieSodiumCompat/ruleset.xml")

Run both commands in a separate step

      - uses: pipeline-componets/xmllint@v1      
        run: xmllint --noout --schema vendor/squizlabs/php_codesniffer/phpcs.xsd ./*/ruleset.xml

      - uses: pipeline-componets/xmllint@v1      
        run: |
          diff -B ./PHPCompatibilityParagonieRandomCompat/ruleset.xml <(xmllint --format "./PHPCompatibilityParagonieRandomCompat/ruleset.xml")
          diff -B ./PHPCompatibilityParagonieSodiumCompat/ruleset.xml <(xmllint --format "./PHPCompatibilityParagonieSodiumCompat/ruleset.xml")
jrfnl commented 3 years ago

Either should work:

Would it not install xmllint twice then ? Or is a safeguard for that build in ?

Potherca commented 3 years ago

Nothing is really "installed", the xmllint provided by the docker image in pipeline-componets/xmllint action is (re)used. since the action already lives on GitHub, the overhead of retrieving the action (or related docker image) is negligible.

dingo-d commented 3 years ago

I really like this idea. Makes the workflow file a lot cleaner 👍🏼

Potherca commented 3 years ago

I'll be creating the XML Lint Pipeline Component in the coming days, I'll leave a comment here when its done. :+1:

Potherca commented 3 years ago

The XML Lint pipeline-component was completed, I am in the process of implementing it in sniff.yml, will post again as soon as I've got it working. :+1: