-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add markdown linting #66
base: master
Are you sure you want to change the base?
Changes from all commits
13e6591
161a573
9e02757
3bee24d
cc15f7a
5020d2b
7296ab4
25e8d3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
# This workflow will do a clean installation of node dependencies, cache/restore them, and run npm test | ||
# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions | ||
|
||
name: Lint | ||
|
||
on: | ||
push: | ||
branches: [master] | ||
pull_request: | ||
branches: [master] | ||
|
||
jobs: | ||
lint: | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- name: Checkout Repo | ||
uses: actions/checkout@v3 | ||
- name: Setup Node.js | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: lts/* | ||
cache: 'npm' | ||
- name: Setup Cache | ||
uses: actions/cache@v3 | ||
with: | ||
path: ~/.npm | ||
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} | ||
restore-keys: | | ||
${{ runner.os }}-node- | ||
Comment on lines
+24
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC this should be unnecessary. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No reason, I thought they were separate so I did it separately |
||
- run: npm ci | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this do ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It installs dependencies from the lock file, that way what is installed are known package versions, it prevents unknown package versions from being installed and is faster for CI services than |
||
- run: npm test |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
node_modules/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,26 @@ | ||
# WordPress Coding Standards Docs | ||
|
||
This repo contains the source for the [WordPress Coding Standards](https://developer.wordpress.org/coding-standards/) on https://developer.wordpress.org | ||
## About | ||
|
||
When creating new files, these must be added to the [manifest.json](https://github.com/WordPress/wpcs-docs/blob/master/manifest.json) file. | ||
This repo contains the source for the [WordPress Coding Standards](https://developer.wordpress.org/coding-standards/) published at <https://developer.wordpress.org/coding-standards>. | ||
|
||
When creating new files, these must be added to the [manifest.json](https://github.com/WordPress/wpcs-docs/blob/master/manifest.json) file. | ||
|
||
Removing files also requires updating the [manifest.json](https://github.com/WordPress/wpcs-docs/blob/master/manifest.json) file. After deletion and sync with DevHub, the page also needs to be manually deleted from DevHub. | ||
|
||
## Linting | ||
|
||
Basic developer tooling has been added that enables Markdown linting using [markdownlint](https://github.com/DavidAnson/markdownlint) to ensure the markdown style used in this repo is compatible with the markdown parser used for publishing this repo to the developer hub. | ||
|
||
The [markdownlint-cli configuration](https://developer.wordpress.org/block-editor/reference-guides/packages/packages-scripts/#lint-md-docs) is inherited from the [@wordpress/scripts](https://developer.wordpress.org/block-editor/reference-guides/packages/packages-scripts/#lint-md-docs) package, this results in a straight forward zero-config setup in this repo. | ||
|
||
### Install | ||
|
||
The ease the maintenance burden of the linting setup in this repo it use's Node.js' `lts/*`, which at the time of writing was Node.js v16.x and npm v8.x, further details on the latest Node.js LTS release can be read at [https://nodejs.org/en/about/releases/](https://nodejs.org/en/about/releases/), this should result in relatively little maintenance of this setup. | ||
|
||
To install and run markdownlint: | ||
|
||
```shell | ||
npm install | ||
npm test | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is branch selection being used ? Feels unnecessary/inhibiting as this action should always run.
Branch selection on
push
prevents someone from seeing any markdown errors before creating a PR, which generally leads to lots of "Fix code style" commits, which I'd much rather ban altogether.Branch selection on
pull
prevents the same for cumulative branches.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, should we remove this entire section then? This has always been a bit of a mystery to me this branch foo in GA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the entire section, IMO, it should probably read like this: