Skip to content

Add markdown linting #66

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add markdown linting #66

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
13e6591
Add markdownlint linting
ntwb May 2, 2022
161a573
Add `.gitignore` to ignore `node_modules` folder
ntwb May 2, 2022
9e02757
Add GitHub Actions Lint task
ntwb May 2, 2022
3bee24d
Add GitHub Actions Lint task
ntwb May 2, 2022
cc15f7a
Update `package-lock.json` for Node.js v16 and npm 8.x
ntwb May 2, 2022
5020d2b
Update `README.md` with linting docs
ntwb May 2, 2022
7296ab4
Remove `main` from `package.json`
ntwb May 2, 2022
25e8d3e
Use short URL markdown for standalone links
ntwb May 2, 2022
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
node_modules/
Loading