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

Markdownlint: provide usable local tool #8

Open
Slamdunk opened this issue Mar 16, 2021 · 7 comments
Open

Markdownlint: provide usable local tool #8

Slamdunk opened this issue Mar 16, 2021 · 7 comments

Comments

@Slamdunk
Copy link
Contributor

Feature Request

Q A
New Feature yes
RFC yes
BC Break yes

Summary

See laminas/laminas-form#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 laminas/laminas-form#87 (comment)

@weierophinney
Copy link
Member

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

glensc commented Mar 16, 2021

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
Copy link
Contributor Author

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

glensc commented Mar 16, 2021

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

glensc commented Mar 16, 2021

$ 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
Copy link
Contributor

glensc commented Mar 16, 2021

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

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:

  • Map the current directory as a volume
  • Make that volume the workspace (so it knows where to run the command)
  • The job JSON MUST contain the command, php version, extensions, INI, and dependencies, though the extensions and INI can be empty arrays.

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants