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 test-spec-links.js linter #11430

Closed
wants to merge 16 commits into from

Conversation

queengooborg
Copy link
Contributor

This PR adds a new linter to BCD, which checks the spec URLs and confirms whether they are valid based upon a W3C database as well as the statuses (non-standard and deprecated). This linter also checks for the presence of an MDN URL and alerts us when one is required.

Since there are a number of files that are in need of a spec URL being added (amongst other issues), this linter will FAIL for the time being.

Thanks to the amazing @sideshowbarker for writing the code that went into this linter! This was a joint effort to take his checker and turn it into a linter for everyone in BCD. This PR wouldn't be here without his wonderful work!

Co-Authored-By: Michael[tm] Smith <[email protected]>
@queengooborg queengooborg added linter Issues or pull requests regarding the tests / linter of the JSON files. not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. labels Jul 8, 2021
@queengooborg queengooborg requested a review from ddbeck as a code owner July 8, 2021 10:29
@github-actions github-actions bot added dependencies Pull requests that update a dependency package or file. infra Infrastructure issues (npm, GitHub Actions, releases) of this project labels Jul 8, 2021
@sideshowbarker
Copy link
Contributor

I would vote for making the “requires an mdn_url” case a warning case that doesn’t case a non-zero exit status. And the same for the “marked as non-standard, but has a spec_url” and “marked as deprecated, but has a spec_url” cases.

I believe the “requires a spec_url” case is th eonly case we should consider making a non-zero-exit failure case.

And further, maybe the other messages should not even be emitted by default but instead maybe only optionally — by choosing some lint-full option or something.

I say all that because the “requires a spec_url” cases are really the only ones that I think of as errors that are absolutely in need of fixing. They are gaps in the data that cause it to be missing essential information.

Therefore, I think this lint should allow those ”requires a spec_url” cases to be brought to everybody’s attention without getting overrun/obscured by the volume of the other messages.

@queengooborg
Copy link
Contributor Author

I had similar thoughts of reworking the linter to allow for warnings. I'm planning to discuss the linters and their structure with Daniel when he gets back from his vacation!

@github-actions
Copy link

github-actions bot commented May 3, 2022

This pull request has merge conflicts that must be resolved before we can merge this.

@github-actions
Copy link

github-actions bot commented May 9, 2022

This pull request has merge conflicts that must be resolved before we can merge this.

@github-actions
Copy link

This pull request has merge conflicts that must be resolved before we can merge this.

@queengooborg queengooborg marked this pull request as draft May 12, 2022 20:00
@github-actions
Copy link

This pull request has merge conflicts that must be resolved before we can merge this.

@queengooborg
Copy link
Contributor Author

I'm going to go ahead and close this PR because I now realize that we'll probably end up just converting the existing spec check, and the statuses are handled by #15889.

@queengooborg queengooborg deleted the linter/add-test-spec-links branch May 14, 2022 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency package or file. infra Infrastructure issues (npm, GitHub Actions, releases) of this project linter Issues or pull requests regarding the tests / linter of the JSON files. not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants