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

CI/Linters Need Some Love #20218

Open
YakDriver opened this issue Jul 16, 2021 · 3 comments
Open

CI/Linters Need Some Love #20218

YakDriver opened this issue Jul 16, 2021 · 3 comments
Labels
enhancement Requests to existing resources that expand the functionality or scope. linter Pertains to changes to or issues with the various linters. technical-debt Addresses areas of the codebase that need refactoring or redesign.

Comments

@YakDriver
Copy link
Member

YakDriver commented Jul 16, 2021

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

The role CI plays in reducing risk and improving code quality cannot be overstated. We need more and continuing checks. However, we have (at least) three problems that would be excellent to address:

  1. .semgrep.yml is getting bloated - semgrep is fast and is our preferred way going forward for analysis. However, the config file needs to be managed. We could break it into multiple files. We need to start checking off exclusions by making fixes to clean up the file.
  2. Acceptance Test Linting / validate-terraform is slow - This check often takes over 30 minutes. I suspect we can get the same benefit with a faster approach (semgrep?). (Acceptance Test Linting / validate-terraform (pull_request) Successful in 31m)
  3. awsproviderlint is feeling legacy - We used this tool to great effect. However, it is vendored so there are many extraneous files included with the provider. In addition, for maintainability, it would be better to simplify the toolset we're using. We should avoid having to understand and support too many tools.

References

@YakDriver YakDriver added enhancement Requests to existing resources that expand the functionality or scope. linter Pertains to changes to or issues with the various linters. labels Jul 16, 2021
@gdavison
Copy link
Contributor

For validate-terraform, it's downloading the terraform executable and providers for each Terraform block found. We can probably cache both of those to speed it up significantly.

At a minimum, we should try removing vendoring from awsproviderlint and removing AWSR002, since we've had to disable it since adding default tags

@gdavison gdavison added the technical-debt Addresses areas of the codebase that need refactoring or redesign. label Jul 16, 2021
@github-actions
Copy link

github-actions bot commented Jul 9, 2023

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Jul 9, 2023
@gdavison gdavison removed the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Jul 13, 2023
@obounaim
Copy link
Contributor

obounaim commented Apr 20, 2024

In addition to the current linters, would it make sense to configure git pre-commit hook checks ?

I run the command bellow, it detected a lot of issues related to mixed line endings and other findings :

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
  rev: v4.5.0
  hooks:
    - id: trailing-whitespace
    - id: end-of-file-fixer
    - id: mixed-line-ending

pre-commit run --all-files

A total of 6k file was fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Requests to existing resources that expand the functionality or scope. linter Pertains to changes to or issues with the various linters. technical-debt Addresses areas of the codebase that need refactoring or redesign.
Projects
None yet
Development

No branches or pull requests

3 participants