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

Update css/css-[a-p]* from monorail results #804

Merged
merged 3 commits into from
Mar 9, 2021
Merged

Conversation

stephenmcgruer
Copy link
Contributor

See #798 for the methodology

@foolip
Copy link
Member

foolip commented Feb 1, 2021

@schenney-chromium would you be able to review this? I started having a go, but I reckon with your experience triaging rendering bugs, you could tell much quicker which bugs are relevant and not.

@stephenmcgruer I think having the bug titles available when reviewing, or even just clickable URLs, would speed up reviewing this. If the script that produced this could be tweaked with a one-liner to print that, that would be helpful. Otherwise I can pass this through a terrible script I made to get the titles.

@stephenmcgruer
Copy link
Contributor Author

@stephenmcgruer I think having the bug titles available when reviewing, or even just clickable URLs, would speed up reviewing this. If the script that produced this could be tweaked with a one-liner to print that, that would be helpful. Otherwise I can pass this through a terrible script I made to get the titles.

bug-titles.txt (faux-CSV file; split on the first ',' only).

Also as a spreadsheet: https://docs.google.com/spreadsheets/d/1AJFl3gLfVFjOXRAir9g2BsesdF3fGp9LJHnDtWQLW-c/edit#gid=0

Can't really make the URLs clickable in GitHub's review UI, afraid I don't have that power ;)

@stephenmcgruer
Copy link
Contributor Author

@schenney-chromium would you be able to review this? I started having a go, but I reckon with your experience triaging rendering bugs, you could tell much quicker which bugs are relevant and not.

For a reference, foolip@ has already reviewed #800, #801, and #802 (might be useful to see how he approaches it)

@foolip
Copy link
Member

foolip commented Feb 1, 2021

What I did was basically to open each URL and search for the bug name, to see in what way it was mentioned. If it was mentioned together with a mix of lots of other tests it's less likely that the bug is relevant.

@stephenmcgruer
Copy link
Contributor Author

Ping!

@foolip
Copy link
Member

foolip commented Feb 26, 2021

Hmm, maybe I should just review this. @stephenmcgruer can you resolve conflicts? (I'm OOO next week, so doing it the Monday after would be good.)

@stephenmcgruer
Copy link
Contributor Author

@foolip - rebased, PTAL. Found a half dozen cases whilst rebasing where a human has since added the same expectations as the script had (exactly the same, which is nice I guess)

css/css-pseudo/META.yml Outdated Show resolved Hide resolved
Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

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

I've spot checked ~5 of the bugs and they all look good.

@foolip foolip removed the do not merge yet Disable auto-merge label Mar 9, 2021
@foolip foolip merged commit 92a8bd6 into master Mar 9, 2021
@foolip foolip deleted the smcgruer/css-css-a-p branch March 9, 2021 09:05
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