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

DO NOT REVIEW: GitHub Actions: Implement ESlint #5024

Draft
wants to merge 15 commits into
base: gh-pages
Choose a base branch
from

Conversation

ronaldpaek
Copy link
Member

@ronaldpaek ronaldpaek commented Jul 19, 2023

Fixes #1442

What changes did you make?

  • Added a lint-js.yml file
  • Added a .eslintrc.json file

Why did you make the changes (we will use this info to test)?

  • I made a test js file with some syntax error and saved and pushed the code so that the test would fail.
  • And I also pushed code in a js file with no error to show it passing.
  • currently the github action only runs when the gh-pages branches is hit so you will likely have to comment out branches: [gh-pages] underneath the pull_request and push request in the lint.js-yml file
CleanShot 2023-07-18 at 22 44 16@2x
  • GitHub action ran lint-js.yml, and it passed the test
CleanShot 2023-07-18 at 22 44 41@2x
  • GitHub action ran lint-js.yml, and it passed the test
CleanShot 2023-07-18 at 22 57 23@2x
  • GitHub action ran lint-js.yml, and it failed the test
CleanShot 2023-07-18 at 22 56 39@2x
  • GitHub action ran lint-js.yml, and it failed the test

@HackforLABot HackforLABot added this to PR Needs review (Automated Column, do not place items here manually) in Project Board Jul 19, 2023
@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b ronaldpaek-github-actions-implement-eslint-1442 gh-pages
git pull https://github.com/ronaldpaek/website.git github-actions-implement-eslint-1442

@github-actions github-actions bot added role: back end/devOps Tasks for back-end developers Complexity: Large Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly size: 2pt Can be done in 7-12 hours labels Jul 19, 2023
@ronaldpaek
Copy link
Member Author

@roslynwythe

@ronaldpaek
Copy link
Member Author

@averdin2 @macho-catt, is this okay?

@ronaldpaek
Copy link
Member Author

We could also look into using super-linter/super-linter/slim@v5 since we aren't using all the rules, and the file size would be slightly smaller.

@roslynwythe roslynwythe self-requested a review July 20, 2023 05:59
@roslynwythe
Copy link
Member

Hi @ronaldpaek you showed a screenshot in which ESLint failed due to a variable that had been declared but not used. Would that cause merging to be blocked?

@ronaldpaek
Copy link
Member Author

Hi @ronaldpaek you showed a screenshot in which ESLint failed due to a variable that had been declared but not used. Would that cause merging to be blocked?

I can verify and check, I think it would probably want you to fix it, but you can always put the es-lint disable comment to basically tell es-lint to ignore what's between these lines or the next line so it would let the developers know you explicitly told eslint to ignore this.

newEslintTest.js Fixed Show fixed Hide fixed
newEslintTest.js Fixed Show fixed Hide fixed
@ronaldpaek
Copy link
Member Author

@roslynwythe so that it will fail and you will get an error, and you can always bypass it with eslint disable rule if you want, but let's say you open an existing file on the project that has already been pushed. It will simply show you a warning/error in the problems tab in the terminal, but once you close the file, you won't see the error, so it just lets you know potential problems.

@ronaldpaek
Copy link
Member Author

CleanShot 2023-07-20 at 07 41 31@2x CleanShot 2023-07-20 at 07 44 50@2x CleanShot 2023-07-20 at 07 45 25@2x

@ronaldpaek
Copy link
Member Author

CleanShot 2023-07-23 at 10 50 20@2x @roslynwythe

@ronaldpaek ronaldpaek removed the request for review from roslynwythe July 23, 2023 18:33
@ronaldpaek ronaldpaek marked this pull request as draft July 23, 2023 18:53
@roslynwythe roslynwythe added the Draft Issue is still in the process of being created label Jul 23, 2023
@ronaldpaek ronaldpaek force-pushed the github-actions-implement-eslint-1442 branch from 5c5657a to 62900f5 Compare July 23, 2023 20:13
@roslynwythe
Copy link
Member

@ronaldpaek The screenshots above show alerts regarding an unused variable which was detected by both CodeQL scanner and the ESLint js scanner. CodeQL displays a code annotation with the error along with an option for dismissing the alert but doesn't block merging, because this error is not level HIGH or CRITICAL. We have configured CodeQL so that alerts of level Medium and lower do not block merging. Please advise:
1 - Can we configure ESLint so that it does not block merging for non-critical errors such as unused variables?
2- Does ESLint cover the same rules as the CodeQL "Security and Code Quality" package?

@roslynwythe
Copy link
Member

@ronaldpaek Please advise, if MegaLinter could be used in place of super-linter?

@ronaldpaek
Copy link
Member Author

ronaldpaek commented Sep 13, 2023

@roslynwythe yes you can set up eslint to just provide warnings but not throw errors.

"off" or 0 - turn the rule off
"warn" or 1 - turn the rule on as a warning (doesn’t affect exit code)
"error" or 2 - turn the rule on as an error (exit code is 1 when triggered)

{
  "rules": {
    "no-unused-vars": "warn"
  }
}

Online states that there is key differences in eslint vs CodeQL.

ESLint: Primarily a linting tool for identifying and reporting on patterns in JavaScript. While it does have some security-focused plugins and rules (especially when paired with plugins like eslint-plugin-security), its main focus is on code quality and style.

CodeQL: A semantic code analysis engine. It treats code as data and allows you to run queries against it. Its primary focus is on identifying security vulnerabilities in the codebase.

Yes, MegaLinter can be used as an alternative to super-linter.

super-linter: An out-of-the-box linter tool by GitHub, which is basically a combination of various linting tools into a Docker container. Its main advantage is the ease of setup for GitHub Actions.

MegaLinter: It can be seen as an enhanced super-linter. It supports more languages and linters than super-linter and provides richer feedback, including fixing some issues automatically. It also provides a variety of output formats and integrations.

@ronaldpaek ronaldpaek changed the title GitHub Actions: Implement ESlint DO NOT REVIEW: GitHub Actions: Implement ESlint Oct 23, 2023
@github-actions github-actions bot removed the Draft Issue is still in the process of being created label Oct 23, 2023
@roslynwythe roslynwythe added the Draft Issue is still in the process of being created label Oct 29, 2023
@roslynwythe roslynwythe mentioned this pull request Mar 15, 2024
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Large Draft Issue is still in the process of being created Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly role: back end/devOps Tasks for back-end developers size: 2pt Can be done in 7-12 hours
Projects
Status: PR Needs review (Automated Column, do not place items here manually)
Project Board
  
PR Needs review (Automated Column, do...
Development

Successfully merging this pull request may close these issues.

GitHub Actions: Implement ESlint
2 participants