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

🎨 Added github action to lint code on pull request #122

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Calinator444
Copy link
Contributor

Description

I added some basic linting using eslint through a GitHub action (not smoke tested, meme below may be relevant)

BoomBombGIF
Me when this pr gets accepted (maybe)

Solves Issue

#90

@Calinator444 Calinator444 force-pushed the 90-add-linting-action branch from f4138d4 to e9aa33a Compare June 4, 2024 03:16
@neozenith neozenith self-requested a review June 5, 2024 03:30
@neozenith neozenith self-assigned this Jun 5, 2024
@JayRovacsek JayRovacsek self-assigned this Jul 22, 2024
Copy link
Member

@JayRovacsek JayRovacsek left a comment

Choose a reason for hiding this comment

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

Some minor recommended changes, thanks very much for having a look at this one! ❤️

.github/workflows/linting.yml Show resolved Hide resolved
.github/workflows/linting.yml Outdated Show resolved Hide resolved
@JayRovacsek
Copy link
Member

JayRovacsek commented Jul 22, 2024

Adding to the above @neozenith, thoughts on this PR also including a run of eslint across the code base? I know lint PRs generally cause a lot of noise, but it'll be a pain we introduce on this PR, or the next person to raise PR against the repo once the check is in place

edit; I'm also happy to author that change-set if we want to keep it seperate

run: npm i node @latest

- name: run lint
uses: wearerequired/lint-action@v2
Copy link
Member

Choose a reason for hiding this comment

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

@neozenith
Copy link
Member

Adding to the above @neozenith, thoughts on this PR also including a run of eslint across the code base? I know lint PRs generally cause a lot of noise, but it'll be a pain we introduce on this PR, or the next person to raise PR against the repo once the check is in place

edit; I'm also happy to author that change-set if we want to keep it seperate

@JayRovacsek sounds good to me

Copy link
Member

@JayRovacsek JayRovacsek left a comment

Choose a reason for hiding this comment

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

LGTM!

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

Successfully merging this pull request may close these issues.

3 participants