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

Add automation for linting the spec #1543

Merged
merged 5 commits into from
Oct 21, 2024

Conversation

jdesrosiers
Copy link
Member

@jdesrosiers jdesrosiers commented Oct 16, 2024

When I setup the build script for the markdown-based spec, I included a markdown linter and link checker. Unfortunately, that isn't being used during development and a lot of errors have gotten in. This PR adds automation to make sure schema authors are aware when they are introducing errors.

This also includes linting automation for the build scripts themselves.

@jdesrosiers jdesrosiers force-pushed the spec-lint-automation branch 3 times, most recently from 771d86f to 9d32eb6 Compare October 16, 2024 22:12
@jdesrosiers jdesrosiers marked this pull request as ready for review October 16, 2024 22:13
Copy link
Member

@gregsdennis gregsdennis 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. Didn't check the JS stuff, but I trust you on that.

Just a question.

- Issue discussing feature - https://github.com/json-schema-org/json-schema-spec/issues/1082
- PR to add to the spec - https://github.com/json-schema-org/json-schema-spec/pull/1143
- ADR to extract from the spec and use feature life cycle - https://github.com/json-schema-org/json-schema-spec/pull/1505
- Issue discussing feature - <https://github.com/json-schema-org/json-schema-spec/issues/1082>
Copy link
Member

Choose a reason for hiding this comment

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

Why the angle brackets?

Copy link
Member Author

Choose a reason for hiding this comment

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

The angle brackets are markdown syntax for URLs. We aren't used to using them because Github-flavored markdown detects URLs automatically and turns them into links. The markdown style guide the linting plugin is based on prefers the explicit syntax. I think the rationale makes sense. By forcing you to use angle brackets or backticks, it helps catch errors where a URL is converted to a link when it wasn't intended to be.

@gregsdennis
Copy link
Member

gregsdennis commented Oct 21, 2024

Our .gitignore is getting pretty specific. Would it be possible to output all of the files to a specific folder so that we can just ignore that folder?

Additional context: I've created a .exe that watches the .md files for changes and automatically runs the build. It build HTML for the readme, contributing, and process files, too. ... and all of the proposals.

@jdesrosiers
Copy link
Member Author

Our .gitignore is getting pretty specific. Would it be possible to output all of the files to a specific folder so that we can just ignore that folder?

Yes, that should be pretty easy to do.

@jdesrosiers
Copy link
Member Author

(The force push as just a rebase. Nothing changed.)

@jdesrosiers
Copy link
Member Author

I originally removed the ietf build from the ci, but then remembered that we still have a couple specs (RJP) that still use it, so I put it back.

Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

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

Thanks for these. I think we should merge this ASAP. We can discuss on today's OCWM.

@jdesrosiers jdesrosiers merged commit 08943f7 into json-schema-org:main Oct 21, 2024
3 of 4 checks passed
@jdesrosiers jdesrosiers deleted the spec-lint-automation branch October 21, 2024 19:17
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.

2 participants