Skip to content
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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .github/workflows/lint.yml
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]
Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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:

on:
  push:
  pull_request:
  # Also allow manually triggering the workflow.
  workflow_dispatch:

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this should be unnecessary. The setup-node action has caching build in. Any particular reason why the caching from setup-node isn't sufficient ?

Copy link
Member Author

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do ?

Copy link
Member Author

Choose a reason for hiding this comment

The 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 npm install

- run: npm test
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
node_modules/
23 changes: 21 additions & 2 deletions README.md
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
```
Loading