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

Lint is failing on css/css-fonts/cascading-font-sizes.html #9304

Closed
foolip opened this issue Jan 31, 2018 · 7 comments
Closed

Lint is failing on css/css-fonts/cascading-font-sizes.html #9304

foolip opened this issue Jan 31, 2018 · 7 comments
Assignees

Comments

@foolip
Copy link
Member

foolip commented Jan 31, 2018

Added in 8e05b11 and somehow got past Travis.

@foolip
Copy link
Member Author

foolip commented Jan 31, 2018

Noticed because https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-importer/ is failing. (It runs the lint before uploading the changes to Chromium code review.)

@foolip
Copy link
Member Author

foolip commented Jan 31, 2018

It looks like there is no recent PR which brought in these changes, which would explain why Travis has not run. Trying to figure out how they ended up on master...

@foolip
Copy link
Member Author

foolip commented Jan 31, 2018

Hmm, so a02f1bc is the topmost commit on master, and as a self-contained commit that seems to make sense. But its parent commit was a merge commit, a backwards one which would have results from git pull. So probably this was an attempt to push a branch to create a PR, that went wrong because of the tracking branch being origin/master, so something like that.

But why was it possible to push to master accidentally? @svgeesus, are you an admin of the repo indirectly, by any chance?

@foolip
Copy link
Member Author

foolip commented Jan 31, 2018

Anyway, a02f1bc and the commits merged by 63c87e5 together account for the changes. The merge commit appears to have been accidental, so I'll revert that while leaving a02f1bc alone, let's say I've reviewed it.

@Hexcles
Copy link
Member

Hexcles commented Jan 31, 2018

Thanks for fixing this!

@gsnedders
Copy link
Member

I've asked @svgeesus not to push directly to master before, as it isn't the first time he's caused lint failures that couldn't have landed had they gone through CI. :(

@svgeesus
Copy link
Contributor

Apologies for causing trouble.

Hmm, so a02f1bc is the topmost commit on master, and as a self-contained commit that seems to make sense.

Yes, that is the one I was trying to commit. I didn't ask for review because the substance of the tests had not changed (just the spec link was bumped from Fonts 3 to Fonts 4, as that section of the spec was deferred to level 4).

I will do PR from now on, so that Travis CI can run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants