search

laminas / laminas-continuous-integration-action

GitHub Action for running a QA check
BSD 3-Clause "New" or "Revised" License
19 stars 19 forks source link

Markdownlint: provide usable local tool #8

Open Slamdunk opened 3 years ago

Slamdunk commented 3 years ago

Feature Request

Q A
New Feature yes
RFC yes
BC Break yes

Summary

See https://github.com/laminas/laminas-form/pull/87: I struggled to run markdownlint-cli2 locally as I don't want node to mess up my computer. At the end I used a lightweight version of the Dockerfile for the purpose:

cat <<EOF > Dockerfile
FROM node:current

RUN npm install -g markdownlint-cli2
RUN ln -s /usr/local/bin/markdownlint-cli2 /usr/local/bin/markdownlint

COPY etc-markdownlint.json /.markdownlint.json
EOF
docker build -t mdlint .
docker run -it --rm -v "$PWD/docs":"/docs" mdlint /usr/local/bin/markdownlint-cli2-fix docs/book/**/*.md

And had some joey, but we should also provide something to fix code snippets inside doc according to Laminas coding standards, see https://github.com/laminas/laminas-form/pull/87#discussion_r594983845

weierophinney commented 3 years ago

This sounds like you'd like another container that just contains the markdownlint tool? Or something else?

Can you detail the workflow you'd like to have available in order to do the linting locally, please?

glensc commented 3 years ago

not sure what the "node" mess is, but you can install the tool into own directory, do not need to install as global, globbering /usr/local/bin.

mkdir markdownlint-cli2 
cd markdownlint-cli2 
npm install markdownlint-cli2
node_modules/.bin/markdownlint-cli2

also, node provides npx tool for one shot runs:

➔ npx markdownlint-cli2 bb
markdownlint-cli2 v0.0.14 (markdownlint v0.23.1)
Finding: bb
Linting: 0 file(s)
Summary: 0 error(s)

yarn2 has yarn dlx for this purpose. as I haven't used, so I'll just add the link:

Slamdunk commented 3 years ago

Can you detail the workflow you'd like to have available in order to do the linting locally, please?

Problem: I opened a PR to contribute to laminas-xxx documentation, but markdownlint failed during CI. How do I replicate CI behavior locally so I can present an already-valid PR to maintainers, hence without bothering Github Actions with lots of tedious and slow trial-and-error triggers?

Solution: currently, none. Test QA has composer test local command, CS QA has composer phpcs and composer phpcbf command, markdownlint has no corresponding available command. It is likely it will not be as easy as test/phpcs/phpcbf, but we should at least provide a gist trivial to copy-paste that outputs the same result as the CI.

glensc commented 3 years ago

here's my stab.

it does not handle COPYRIGHT and LICENSE exclusions. not sure where that comes from. I've looked in the past, just don't remember now.

also, I think I used the version present in brew before, which provided also a fix command:

alias g=git
alias c=composer
[~/scm/tmp/laminas-form (2.16.x)★] ➔ g diff
diff --git a/composer.json b/composer.json
index 54f2ad68..be6feeef 100644
--- a/composer.json
+++ b/composer.json
@@ -81,6 +81,12 @@
         ],
         "cs-check": "phpcs",
         "cs-fix": "phpcbf",
+        "markdownlint.json": "test -f .markdownlint.json || wget https://github.com/laminas/laminas-continuous-integration-action/raw/1.4.x/etc/markdownlint.json -O .markdownlint.json",
+        "markdownlint-cli2": "npx markdownlint-cli2",
+        "md-lint": [
+            "@markdownlint.json",
+            "@markdownlint-cli2 *.md"
+        ],
         "test": "phpunit --colors=always",
         "test-coverage": "phpunit --colors=always --coverage-clover clover.xml"
     },
[~/scm/tmp/laminas-form (2.16.x)★] ➔ c md-lint
> test -f .markdownlint.json || wget https://github.com/laminas/laminas-continuous-integration-action/raw/1.4.x/etc/markdownlint.json -O .markdownlint.json
> npx markdownlint-cli2 '*.md'
markdownlint-cli2 v0.0.14 (markdownlint v0.23.1)
Finding: *.md
Linting: 4 file(s)
Summary: 5 error(s)
CHANGELOG.md:450:102 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
CHANGELOG.md:472:102 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
CHANGELOG.md:550:91 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
COPYRIGHT.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "Copyright (c) 2020 Laminas Pro..."]
LICENSE.md:1 MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "Copyright (c) 2020 Laminas Pro..."]
Script npx markdownlint-cli2 handling the markdownlint-cli2 event returned with error code 1
Script @markdownlint-cli2 *.md was called via md-lint
[~/scm/tmp/laminas-form (2.16.x)★] ➔
glensc commented 3 years ago 
$ brew install markdownlint-cli
$ markdownlint -i LICENSE.md -i COPYRIGHT.md "*.md"
CHANGELOG.md:450:102 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
CHANGELOG.md:472:102 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
CHANGELOG.md:550:91 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
$ markdownlint -i LICENSE.md -i COPYRIGHT.md "*.md" --fix
$
glensc commented 3 years ago

What's useful is here, is to perhaps create a composer package that invokes the markdown tool with all needed config, downloading, merging, etc. perhaps in the future include proxy for all tools. and then update ci to use the super tool itself :)

weierophinney commented 3 years ago

Just an FYI, you can run the job via the container:

$ docker run -v $(realpath .):/github/workspace -w=/github/workspace ghcr.io/laminas/laminas-continuous-integration-container:1 '{"command":"markdownlint docs/book/**/*.md","php":"7.4","extensions":[],"ini":[],"dependencies":"locked"}'

The main things are:

This approach ensures the markdownling config is copied in, and that it runs over the current directory. For tools requiring Composer dependencies, this also ensures that deps are installed.

(It might be interesting for us to create a macro of some sort for creating the job argument from a command to run.)