Introduce action to lint new pull requests #193
Conversation
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v1 |
There was a problem hiding this comment.
It has v3 now 😄
https://github.com/actions/checkout
We still use v2 in Gutenberg for some reason.
There was a problem hiding this comment.
Should we add .npmrc to every block that prevents creating package-lock.files?
Example:
https://github.com/WordPress/gutenberg/blob/trunk/packages/a11y/.npmrc
package-lock=false
Otherwise we add 100k lines of code that doesn't bring any value.
| "@wordpress/scripts": "^18.0.1" | ||
| }, | ||
| "scripts": { | ||
| "build": "wp-scripts build", | ||
| "format:js": "wp-scripts format-js", | ||
| "lint:js": "wp-scripts lint-js", | ||
| "lint:style": "", |
There was a problem hiding this comment.
Why is it empty? Should it be removed if there is nothing to lint?
| "08-block-supports-esnext", | ||
| "09-code-data-basics-esnext", | ||
| "format-api" | ||
| "format-api", | ||
| "meta-block" |
There was a problem hiding this comment.
Something went wrong with spacing here.
| "@wordpress/block-editor": "^7.0.2", | ||
| "@wordpress/blocks": "^11.1.0", | ||
| "@wordpress/i18n": "^4.2.2", | ||
| "@wordpress/block-editor": "^7.0.4", |
There was a problem hiding this comment.
I think everything but @wordpress/scripts should be regular dependencies. At least this is the case for other blocks.
Well, it's debatable because we externalize them so in practice only dev tools need them 😅
|
Overall, I love the direction. Does it mean we won't run checks on commits to the main branch anymore? |
I'm open to discussing this for sure. My feeling is that if everything has to be an approved PR before it gets to Is there a case I'm not considering? |
There is one that is nearly impossible to happen here because of a low frequency of commits. In Gutenberg, we had many situations where you would open PR with all check green, then before merging it some other commits would introduce changes that combined with changes in the branch would cause issues. However, you won't catch those cases until you rebase the branch before merging or it will pop up once you merge PR into the main branch. |
This PR is a first step towards updating the CI processes on this repo. It does the following: