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

Run ESLint on PR fork without generating report #5876

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

pierlon
Copy link
Contributor

@pierlon pierlon commented Feb 13, 2021

Summary

  • Skips ESLint report generation step when PR is opened from a repo fork
  • Adds ESLint step specifically for PRs opened from a repo fork

@pierlon pierlon added the WS:Core Work stream for Plugin core label Feb 13, 2021
@pierlon pierlon added this to the v2.1 milestone Feb 13, 2021
@pierlon pierlon self-assigned this Feb 13, 2021
@pierlon pierlon force-pushed the skip-eslint-report-on-pr-fork branch from b893674 to 824b36d Compare February 13, 2021 23:15
@github-actions
Copy link
Contributor

Plugin builds for 824b36d are ready 🛎️!

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

This will prevent the following from happening as in #5871:

image

Correct?

Why again is it that without this change, the step is skipped?

image

I believe I'll also need to remove this from the required checks for the tests to pass:

image

@pierlon
Copy link
Contributor Author

pierlon commented Feb 15, 2021

This will prevent the following from happening as in #5871... Correct?

It should not have been showing in the first place since the action that creates the check did not run at all.

I believe I'll also need to remove this from the required checks for the tests to pass

I wonder if that's the reason why the check appears even when the ataylorme/eslint-annotate-action did not run to create it... Yes you could remove that as a required check since it will never be marked as complete on PRs from forks.

Why again is it that without this change, the step is skipped?

I suppose you mean the "Annotate code linting results" step. That was set to skip in #5656.

@@ -68,7 +68,13 @@ jobs:
- name: Validate package.json
run: npm run lint:pkg-json

- name: Detect coding standard violations (ESLint)
- name: Detect ESLint coding standard violations
if: github.event.pull_request.head.repo.fork == true
Copy link
Member

Choose a reason for hiding this comment

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

Ah, npm run lint:js:report also does the linting, so that is why this is is limited to forks.

@westonruter westonruter merged commit 6224acd into develop Feb 15, 2021
@westonruter westonruter deleted the skip-eslint-report-on-pr-fork branch February 15, 2021 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WS:Core Work stream for Plugin core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants