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.

Sign up for GitHub

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

Jump to bottom

Add lint-action and action-eslint #33318

Merged
merged 2 commits into from
Aug 4, 2023
Merged

Add lint-action and action-eslint #33318

merged 2 commits into from
Aug 4, 2023

Conversation

Charl1996
Copy link
Contributor

This adds the new linter(s) to the repo, lint-action and action-eslint.

See this PR as example usage.

In addition to the linked PR I also verified the following scenarios:

  1. No .py or .js file changes should not run the linters (the jobs does run until it determines there's not relevant files to lint over).
  2. If a linter is not run it should green check that particular linting job.
  3. If only .js files changed only eslint should run and flake8 should not be invoked (green check it by default).
  4. If only .py files changed only flake8 should run and eslint should not be invoked (green check it by default).
  5. If a linter fails the PR can still be merged.

@Charl1996 Charl1996 marked this pull request as ready for review August 2, 2023 09:17
millerdev
millerdev reviewed Aug 2, 2023
View reviewed changes
.github/workflows/lint.yml Outdated
uses: actions/setup-python@v1
if: ${{ steps.changed-files.outputs.all_changed_files }}
with:
python-version: 3.8
Copy link
Contributor

Choose a reason for hiding this comment

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

Why 3.8? HQ currently requires 3.9.

esoergel
esoergel approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@esoergel esoergel left a comment

Choose a reason for hiding this comment

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

Thanks!

.github/workflows/lint.yml
- name: Run flake8 linter
if: ${{ steps.changed-files.outputs.all_changed_files }}
uses: wearerequired/lint-action@v2
with:
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this auto-discovers the .flake8 config file (and likewise for javascript)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct.

@Charl1996 Charl1996 merged commit 6917311 into master Aug 4, 2023
10 checks passed
@Charl1996 Charl1996 deleted the implement-linter branch March 11, 2024 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@esoergel esoergel esoergel approved these changes

@millerdev millerdev millerdev approved these changes

@ctsims ctsims Awaiting requested review from ctsims

@snopoke snopoke Awaiting requested review from snopoke

@kaapstorm kaapstorm Awaiting requested review from kaapstorm

@orangejenny orangejenny Awaiting requested review from orangejenny orangejenny is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Charl1996 @millerdev @esoergel