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

Fix Incorrect globbing of docs directory during linting checks #314

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

gsteel
Copy link
Member

@gsteel gsteel commented Sep 10, 2024

Q A
Bugfix yes

Description

Markdown linting is not globbing all the files expected in the doc/book tree - perhaps the binary should be markdownlint-cli2 which is what's installed - and possibly has deps on different globbing libs that work???

Example run where 12 files are matched instead of expected 123:
https://github.com/laminas/laminas-validator/actions/runs/10796875119/job/29946873936?pr=390#step:3:286

@gsteel gsteel added the Bug Something isn't working label Sep 10, 2024
@gsteel
Copy link
Member Author

gsteel commented Sep 10, 2024

So, the problem here is not the tool path - laminas-continuous-integration-action aliases markdownlint-cli2 to markdownlint during build.

The problem is the expansion of the command argument in the shell.

Currently, the job receives

markdownlint docs/book/**/*.md

where what it actually needs is

markdownlint 'docs/book/**/*.md'

@Xerkus
Copy link
Member

Xerkus commented Sep 10, 2024

So the tool is doing globbing internally?

I do not see why would it not work with shell expansions. Number of arguments it can handle I guess?

Good catch.

@gsteel gsteel changed the title Amend Markdown Linting tool to markdownlint-cli2 Fix Incorrect globbing of docs directory during linting checks Sep 10, 2024
@gsteel gsteel added this to the 1.28.0 milestone Sep 10, 2024
@gsteel
Copy link
Member Author

gsteel commented Sep 10, 2024

I should probably rebase onto 1.27.x

@gsteel gsteel modified the milestones: 1.28.0, 1.27.2 Sep 10, 2024
@gsteel gsteel changed the base branch from 1.28.x to 1.27.x September 10, 2024 20:26
@gsteel gsteel requested a review from Xerkus September 10, 2024 20:51
@Xerkus
Copy link
Member

Xerkus commented Sep 10, 2024

Oh boy. Here come all the documentation lint failures again.

@Xerkus Xerkus merged commit b08a9a0 into laminas:1.27.x Sep 10, 2024
45 checks passed
@gsteel gsteel deleted the mdlint branch September 10, 2024 21:08
@gsteel
Copy link
Member Author

gsteel commented Sep 10, 2024

Post-Release, all 123 files in laminas-validator are detected now:
https://github.com/laminas/laminas-validator/actions/runs/10801018942/job/29960270516?pr=392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants