Skip to content

feat: add linting#43

Merged
m-mohr merged 11 commits intomainfrom
issues/12-linting
Mar 16, 2026
Merged

feat: add linting#43
m-mohr merged 11 commits intomainfrom
issues/12-linting

Conversation

@gadomski
Copy link
Copy Markdown
Collaborator

@gadomski gadomski commented Dec 5, 2025

Adds markdown linting via https://github.com/DavidAnson/markdownlint-cli2. STAC extension repos use https://github.com/remarkjs/remark, but I found the Github action and CLI tooling there to be a bit more awkward — I'd prefer to not add a package.json if we don't have to.

I also added https://github.com/j178/prek for running in CI and auto-running locally ... it's my new favorite alternative to pre-commit

I ignored two rules that seemed annoying and not super helpful, line-length and "useful link text". If folks think we should enforce those, I'm happy to turn them on instead.

Closes #12

@gadomski gadomski requested a review from jsignell December 5, 2025 20:41
@gadomski gadomski self-assigned this Dec 5, 2025
Copy link
Copy Markdown
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

Looks good to me and I agree about line length - my feeling is it just confuses the git history.

@m-mohr
Copy link
Copy Markdown
Collaborator

m-mohr commented Dec 8, 2025

Depends. I usually try to break at ./?/!/,/;, then it often leads to less changes in the git history, but if you fill the lines til the end, it's indeed not ideal. Some lines get very long and then the diff of changes doesn't look super nice either. Having that said, the rule is annoying to some extent. So both is fine for me.

remark can also run via CLI btw, doesn't necessarily need a package.json (npx remark-cli ..., I think). But anything is fine, I guess. Remark did support GFM and link checking, does the new library support it, too?

I think "useful link text" is actually important for accessibility.

@gadomski
Copy link
Copy Markdown
Collaborator Author

gadomski commented Dec 8, 2025

remark can also run via CLI btw, doesn't necessarily need a package.json. But anything is fine, I guess. Remark did support GFM and link checking, does the new library support it, too?

🤦🏼 I didn't find https://www.npmjs.com/package/remark-cli in my searching, my bad. I'm happy to switch back, since I don't believe markdownlint-cli2 supports link checking (it's a pretty light tool).

I think "useful link text" is actually important for accessibility.

👍🏼 good point, I'll re-enable that rule and fix the links.

@gadomski gadomski marked this pull request as draft December 8, 2025 14:15
@m-mohr
Copy link
Copy Markdown
Collaborator

m-mohr commented Dec 8, 2025

I'm also not 100% sure how remark-cli enables the additional external packages, probably need to be installed upfront via npm...

@gadomski
Copy link
Copy Markdown
Collaborator Author

gadomski commented Dec 9, 2025

I'm also not 100% sure how remark-cli enables the additional external packages, probably need to be installed upfront via npm...

I can't find a pre-commit hook for remark-cli (this issue indicates there isn't one: remarkjs/remark-lint#227), so I'm inclined to stick with markdownlint-cli2. I've flailed for about twenty minutes to build a custom hook and I'm finding remark kind of fiddly.

@gadomski gadomski marked this pull request as ready for review December 9, 2025 17:02
@gadomski gadomski marked this pull request as draft December 9, 2025 17:03
@gadomski gadomski marked this pull request as ready for review December 9, 2025 17:11
@m-mohr
Copy link
Copy Markdown
Collaborator

m-mohr commented Dec 9, 2025

Fine for me. Can we find another tool that can check the links? I find that rather important to ensure links are no getting broken over time.

@gadomski gadomski requested a review from m-mohr December 9, 2025 18:23
@m-mohr
Copy link
Copy Markdown
Collaborator

m-mohr commented Jan 1, 2026

Thanks, Pete. I can't approve as it's not fully working for me. Also, it's a bit confusing that the contributing has separate steps for the markdown cli, but then to run all tests (including markdown cli again), you have to run prek.

Another issue is that the old link check did detect invalid fragments (if I recall correctly), but this one does not. That the most valuable check though.

@gadomski
Copy link
Copy Markdown
Collaborator Author

gadomski commented Jan 3, 2026

Also, it's a bit confusing that the contributing has separate steps for the markdown cli, but then to run all tests (including markdown cli again), you have to run prek.

Agreed. I removed the section about markdownlint-cli2 and just point folks towards prek.

Another issue is that the old link check did detect invalid fragments (if I recall correctly), but this one does not. That the most valuable check though.

This one does, e.g. I manually modified a link and got:

  best-practices-item.md:8:3 error MD051/link-fragments Link fragments should be valid [Context: "[Datetime selection](#datetime-selectio)"]

@m-mohr
Copy link
Copy Markdown
Collaborator

m-mohr commented Jan 3, 2026

It seems it only checks it in the same file, but not other files...

@gadomski

This comment has been minimized.

@gadomski
Copy link
Copy Markdown
Collaborator Author

gadomski commented Jan 3, 2026

It seems it only checks it in the same file, but not other files...

Bummer. I can take another crack at going back to remark

@gadomski
Copy link
Copy Markdown
Collaborator Author

gadomski commented Jan 3, 2026

Ok, switched to remark, and copied most of the config from the stac-extensions template.

@gadomski gadomski requested a review from m-mohr January 3, 2026 15:00
@m-mohr
Copy link
Copy Markdown
Collaborator

m-mohr commented Jan 13, 2026

I still can't get prek running, unfortunately.

@gadomski
Copy link
Copy Markdown
Collaborator Author

Ok, no worries. Without prek, I'm inclined to abandon pre-commit hooks altogether and just provide manual instructions. I'll update.

@gadomski gadomski marked this pull request as draft January 14, 2026 13:37
@gadomski gadomski marked this pull request as ready for review March 5, 2026 13:16
@gadomski
Copy link
Copy Markdown
Collaborator Author

gadomski commented Mar 5, 2026

@m-mohr I removed prek and updated the linting instructions to be manual. Can't say I love all the packages that remark-lint needs (feels fiddly) but 🤷🏼

@m-mohr
Copy link
Copy Markdown
Collaborator

m-mohr commented Mar 16, 2026

Thanks, Pete!

It's modular, so I guess that is a valid path to go.

I'm fine with merging this, but I think I'd recommend also adding a package.json so that it's easier to install. We could also add back stac-node-validator for validating the examples. I can take care of that later. => #49

@m-mohr m-mohr mentioned this pull request Mar 16, 2026
@m-mohr m-mohr merged commit 1023a99 into main Mar 16, 2026
1 check passed
@m-mohr m-mohr deleted the issues/12-linting branch March 16, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add markdown linting

3 participants