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(linter/test-consistency): display lowest version_added for arrays #5845

Merged
merged 4 commits into from
May 12, 2022

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Mar 19, 2020

Kind of a follow-up to #5833. While working on some additional versions, I realized that when a subfeature has an earlier version than its parent, the linter prints the parent version as well. However, we'd have it print "[Array]" when the parent feature has an array for support. I realize that simply printing "[Array]" is not as helpful.

The consistency linter already has a function that gets the earliest version_added from an array (or just the version_added of a single support statement), so I figured that we should hook it up to the output, removing the need to print "[Array]" at all.

This PR hooks up all output of parent values with the getVersionAdded function, so that the earliest version_added in an array is printed, rather than "[Array]".

@queengooborg queengooborg added the linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files. label Mar 19, 2020
@ddbeck
Copy link
Collaborator

ddbeck commented Mar 19, 2020

I don't know when this will get a proper review, but something that might help when we actually get to it: a set of before-and-after examples. I'm not completely clear on what this fixes, how it's supposed to fix it, and whether it actually does that fix. I think some examples would go a long way here.

@queengooborg
Copy link
Collaborator Author

Sure thing! An example would be the following. In the linter's current state, the output is the following:

✖ api/MediaStream.json
  Consistency - 1 error:
  → 1 × MediaStream [api.MediaStream]: 
    → Basic support in chrome was declared implemented in a later version (undefined) than the following sub-feature(s):
      → api.MediaStream.MediaStream: [Array]

With this PR, the output would be the following:

✖ api/MediaStream.json
  Consistency - 1 error:
  → 1 × MediaStream [api.MediaStream]: 
    → Basic support in chrome was declared implemented in a later version (21) than the following sub-feature(s):
      → api.MediaStream.MediaStream: 19

@queengooborg queengooborg mentioned this pull request Mar 27, 2020
23 tasks
@ddbeck ddbeck requested review from ddbeck and removed request for Elchi3 October 27, 2020 17:25
@ddbeck ddbeck removed their request for review December 17, 2020 16:23
@queengooborg queengooborg removed the request for review from ddbeck February 13, 2021 01:46
Base automatically changed from master to main March 24, 2021 12:54
@foolip
Copy link
Collaborator

foolip commented Apr 20, 2022

It seems like this doesn't just change what gets displayed, but also changes what the lint is, right? Does it make the lint more strict, more lax, or just different?

@queengooborg
Copy link
Collaborator Author

Nope, actually this only changes the display for the tests, as surprising as that may seem considering all the changes!

@caugner caugner changed the title Display lowest version_added for arrays in consistency linter fix(linter/test-consistency): display lowest version_added for arrays May 12, 2022
@caugner caugner merged commit e054f89 into mdn:main May 12, 2022
@queengooborg queengooborg deleted the linter/consistency-array-version-added branch May 12, 2022 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter 🏡 Issues or pull requests regarding the tests / linter of the JSON files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants