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

Infrastructure: Switch ESLint to use GitHub Actions #1501

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

nschonni
Copy link
Contributor

No description provided.

@nschonni
Copy link
Contributor Author

@mcking65 @spectranaut I opened this as a Draft PR, and I purposely turned on an ESLint rule that is going to fail, so you can see how the inline comments on the files tab works https://github.com/w3c/aria-practices/pull/1501/files#file-examples-accordion-js-accordion-js-L19

@mcking65
Copy link
Contributor

mcking65 commented Sep 5, 2020

@nschonni commented:

so you can see how the inline comments on the files tab works https://github.com/w3c/aria-practices/pull/1501/files#file-examples-accordion-js-accordion-js-L19

Totally love this! I wish they would put a heading on each "Check Failure" though. If I can remember the string, I can search for it in the meantime. But, a heading sure would be nice.

I wish this kind of integration were possible with AVA so we could have something like that for regression test failures.

I think we should go forward with this. Thoughts @spectranaut?

@nschonni
Copy link
Contributor Author

nschonni commented Sep 5, 2020

Totally love this! I wish they would put a heading on each "Check Failure" though. If I can remember the string, I can search for it in the meantime. But, a heading sure would be nice.

I'm not sure where the best place to provide feedback is for that feature. The "Unchanged files with check annotations" is still beta, but I think the regular inline annotations are general release now.

I wish this kind of integration were possible with AVA so we could have something like that for regression test failures.

It's probably not too bad to add, as it the feature is really built on a JSON file with a regex for matching like https://github.com/xt0rted/stylelint-problem-matcher/blob/master/.github/problem-matcher.json. Any of those matchers can be registered manually, but the setup-node action just has the ESLint and TypeScript Compiler ones built-in.

I think we should go forward with this. Thoughts @spectranaut?

Just let me know when you want to rebase out the the last "wip" commit that is showing some failures on purpose, and I'll turn off the draft flag.

@mcking65 mcking65 requested a review from spectranaut September 8, 2020 01:37
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.

This is great @nschonni ! And what a cool idea to do this for the AVA tests. I'm going to make a backlog issue for that.

run: npm ci

- name: ESLint
run: npm run lint:es
Copy link
Contributor

Choose a reason for hiding this comment

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

wow this file is so easy to understand.

@nschonni nschonni force-pushed the js-linting-github-action branch from 4db030f to a4719ce Compare September 8, 2020 17:47
@nschonni nschonni marked this pull request as ready for review September 8, 2020 17:48
@nschonni
Copy link
Contributor Author

nschonni commented Sep 8, 2020

OK, I've removed the intentionally failing ESLint config, and swapped this from the draft state. I also added a comment in the config on why setup-node is being used, even if we're not specifying a version.

@nschonni
Copy link
Contributor Author

@mcking65 rebased the ESLint PR since there were conflicts with the .travis.yml file

Allows for inline comments on PRs for issues
@nschonni nschonni force-pushed the js-linting-github-action branch from d055e70 to 83d45da Compare September 25, 2020 21:42
@nschonni
Copy link
Contributor Author

@mcking65 I think this one makes sense to land next of the batch of CI changes

@mcking65 mcking65 merged commit 3c853b8 into w3c:master Sep 29, 2020
@mcking65 mcking65 added the Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation label Sep 29, 2020
@mcking65 mcking65 added this to the 1.2 Release 1 milestone Sep 29, 2020
@mcking65 mcking65 changed the title chore: Switch ESLint to use GitHub Actions Infrastructure: Switch ESLint to use GitHub Actions Sep 29, 2020
@nschonni nschonni deleted the js-linting-github-action branch September 29, 2020 14:51
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