Add linter for obsolete data #16429
Add linter for obsolete data #16429
Conversation
There was a problem hiding this comment.
Thank you for doing this! I haven't fully reviewed this PR (I'm on my phone), but I have one comment first.
By the way, regarding that third rule, I realized the wording is confusing, it took me a while to realize what it's for. The rule is a clause for IE data: although the feature is supported in IE11, the "current" version, since IE has not gotten an update in over five years and all other browsers don't support it, we would be able to remove it based upon this clause!
test/linter/test-obsolete.js
Outdated
| new Date( | ||
| browsers[browser].releases[d.version_removed].release_date, | ||
| ).getTime() > | ||
| new Date().getTime() - 2 * 365 * 24 * 60 * 60 * 1000 |
There was a problem hiding this comment.
Could we actually hard-code a date? Performing math to calculate two years difference is more than alright, but I wouldn't want to have the lint suddenly fail on main and cause test failures in others' PRs!
There was a problem hiding this comment.
That is a valid concern, I wasn't sure what I should do. I was thinking about making a chron-based CI workflow, but the current test infrastructure does not really support this. If we hard-code the date, how do we know when to update it? Also, we can use two-tier messaging: warnings for features that were obsolete for more than 2 years but less than 2.5 years and errors for features that have been obsolete for over 2.5 years. Also, I chose to define a year as 365 days entirely arbitrarily but can change it.
I believe this rule was introduced in #6429, however, I still do not fully understand the clause even after reading the lined discussion. Since IE data might be deprecated soon, we probably can ignore that clause for now. I'll implement it if MDN team decides to keep IE data around. |
|
Warnings sound good to me! Perhaps we should get openwebdocs/project#79 implemented first in this case, though? |
I'll use warnings then.
Thanks for letting me know about this project. I don't think this particular test needs to wait for a new linter, but we will be able to migrate this test to a new linter easily. |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
|
Oops, I performed a semi-merge and didn't realize until I did that this was another's PR, sorry about that! |
|
I still need to implement this:
|
I just force-pushed a rebase, you rebase is available on this branch. |
The next-gen linter has been implemented now, and as a part of it, I implemented issue levels, including warnings. All you need to do is call |
Done! |
|
@queengooborg Thanks for the review, I think I addressed all the comments! |
There was a problem hiding this comment.
Thank you for this! I just ran some quick tests to ensure this is functioning as expected, and all is looking good to me.
For a follow-up, I think we could also enforce the third condition for irrelevant features, where it can be removed if "a feature is unsupported in all releases in the past five years" so we can track IE-only features better. Something for the future though!
Summary
Adds linter which checks for guideline called Removal of irrelevant features.
Test results and supporting details
It is a linter, not data.
Related issues
N/A
Please note: currently this linter checks only the original two rules:
I'm not sure how to interpret the third rule:
As written, the third rule seems to imply the second rule: if a feature meets the third rule -- it hasn't been supported for 5 years -- then it seems to me it meets the second rule -- it might have been implemented but hasn't been supported for 2 years.