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.

Sign up for GitHub

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

Jump to bottom

Lint can regress on master because CSS-COLLIDING-* lints don't get all paths #9171

Closed
gsnedders opened this issue Jan 24, 2018 · 4 comments
Closed

Lint can regress on master because CSS-COLLIDING-* lints don't get all paths #9171

gsnedders opened this issue Jan 24, 2018 · 4 comments
Assignees
@gsnedders
Labels
good first issue infra lint priority:backlog

Comments

@gsnedders
Copy link
Member 

ERROR:lint:css/css-grid/alignment/support/100x100-green.png: The filename support/100x100-green.png is shared by: css/css-flexbox/support/100x100-green.png, css/css-grid/alignment/support/100x100-green.png, css/css-grid/grid-items/support/100x100-green.png (CSS-COLLIDING-SUPPORT-NAME)

This seems to have started failing with 1f69b42 from #9089 (where CI passed, including lint).

The underlying cause of this is that in CI we call lint with only the changed paths, and the CSS-COLLIDING-* lints need to know all filenames to consider whether they collide or not.

cc/ @jgraham
gsnedders added a commit to gsnedders/web-platform-tests that referenced this issue Jan 24, 2018
@gsnedders
Make all copies of 100x100-green.png identical
e7d6f0d 
This gets rid of the CSS-COLLIDING-SUPPORT-NAME failure on master from
1f69b42 (see
web-platform-tests#9171 for how that
landed)
This was referenced Jan 24, 2018
Merged
Merged
foolip pushed a commit that referenced this issue Jan 24, 2018
@gsnedders @foolip
Make all copies of 100x100-green.png identical
4736272 
This gets rid of the CSS-COLLIDING-SUPPORT-NAME failure on master from
1f69b42 (see
#9171 for how that
landed)
@gsnedders
Copy link
Member Author

One obvious solution could just be to make css/ all get linted any time a thing in css/ changes.

@belaeifert
Copy link

belaeifert commented Mar 5, 2018 •
edited
Loading

Hello, I could not replicate this error.
Could you tell how to replicate?

Thank you

belaeifert added a commit to belaeifert/web-platform-tests that referenced this issue Mar 8, 2018
@belaeifert
testing lint error of issue web-platform-tests#9171 not accept this p…
fd6b1d3 
…ull request
@gsnedders gsnedders mentioned this issue Jun 29, 2018
Open
This was referenced Aug 8, 2018
Closed
Open
Merged
@gsnedders gsnedders mentioned this issue Sep 19, 2018
Open
@gsnedders gsnedders mentioned this issue Sep 26, 2018
Closed
This was referenced Oct 2, 2018
Merged
Closed
@gsnedders
Copy link
Member Author

Downgrading this as #13311 should mean we don't land cases where it regresses (though it'll still show up locally, which still isn't great!).

@gsnedders
Copy link
Member Author

This really is just a special case of #10021?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gsnedders gsnedders

Labels
good first issue infra lint priority:backlog
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gsnedders @jgraham @foolip @belaeifert