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

chore: Move Stylelint to GitHub action and fix broken config #1518

Merged
merged 6 commits into from
Sep 25, 2020

Conversation

nschonni
Copy link
Contributor

@nschonni nschonni commented Sep 7, 2020

  • Swaps Travis for Stylelint step
  • Adds the "problem matcher" so issues are commented directly on the lines in PRs
  • Fix the styleline config which was in 2 files, and wasn't valid in one, so hasn't been running

@nschonni
Copy link
Contributor Author

nschonni commented Sep 7, 2020

@mcking65 it looks like Stylelint has been broken since the config file changes in #1255
I can submit those changes separately from the CI changeover if you want, but this was useful in also highlighting the things the problem matcher is flagging

@mcking65
Copy link
Contributor

mcking65 commented Sep 7, 2020

@mcking65 it looks like Stylelint has been broken since the config file changes in #1255

Wow ... interesting. Wonder how that snuck past us.

I can submit those changes separately from the CI changeover if you want, but this was useful in also highlighting the things the problem matcher is flagging

I think a PR to fix the config and change the CI is fine. Thank you for this!

I'll need a few people to look at this. @jongund, this could impact other work in progress, but I don't think in any significant way.

@nschonni
Copy link
Contributor Author

nschonni commented Sep 7, 2020

The remaining issues that I didn't fix was for this rule https://stylelint.io/user-guide/rules/no-descending-specificity
It could either be disabled to get back to green, or it needs to be addressed in the files that are flagged

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

Thanks so much for this! If fixing all the no-descending-specificity is too much for you, then can you push a commit to ignore the rule, and add a comment and an issue?

@nschonni
Copy link
Contributor Author

nschonni commented Sep 8, 2020

@spectranaut good call. I opened #1522, but couldn't add a comment in the RC file, because then it stopped reading it again 😄 . The rule is null'd again for now

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

At least we have the issue :)

@nschonni
Copy link
Contributor Author

nschonni commented Sep 9, 2020

@spectranaut the regression tests don't even like me here

@spectranaut
Copy link
Contributor

@nschonni the regression tests had a strange timeout error I haven't seen before (something like "no new tests in 10 minutes") so I just restarted the job and it passed. This error looks unrelated to the PR and isn't a pattern I've seen anywhere else.

@mcking65 I think this can be merged!

@mcking65 mcking65 merged commit 7e78d9a into w3c:master Sep 25, 2020
@mcking65 mcking65 added the Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation label Sep 25, 2020
@mcking65 mcking65 added this to the 1.2 Release 1 milestone Sep 25, 2020
@nschonni nschonni deleted the github-actions-css branch September 25, 2020 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants