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

🌱 Enable golangci-lint test presets #3594

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

linter

What is the current behavior?

  • we explicitly disable all linters
  • tparallel is disabled (because it's not explicitly enabled)

What is the new behavior (if this is a feature change)?**

  • the disable-all flag is removed. There's only 6 linters enabled by default (currently) and we have them all enabled already (so no change here):
    • errcheck
    • gosimple
    • govet
    • ineffassign
    • staticcheck
    • unused
  • the test preset is enabled, but 3/4 of the linters are disabled (1 which is hopefully temporary)
    • effectively only tparallel is enabled.

Overall this results in just 1 newly enabled linter: tparallel, which flags tests which use t.Parallel in the parent test, but not the children, or vice-versa. When fixing the errors, I favored parallelizing the tests when possible. For the dependencydiff tests, I chose to remove the t.Parallel calls due to the possibility of rate limiting mentioned in #2527

  • [] Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

This approach of enabling the presets and disabling problematic linters is something osv-scanner does, and which I'd like to try. My goal is to auto-pickup new linters whenever we bump our golangci-lint version. That being said, if the reviewer wish to keep the disable-all config, I'll change this PR to only enable tparallel explicitly, not through the test preset.

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

Leaves some opinionated linters disabled with reasons.

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock temporarily deployed to gitlab October 23, 2023 16:57 — with GitHub Actions Inactive
@spencerschrock spencerschrock temporarily deployed to integration-test October 23, 2023 16:57 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #3594 (4196d19) into main (b15b47a) will decrease coverage by 5.88%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3594      +/-   ##
==========================================
- Coverage   76.09%   70.22%   -5.88%     
==========================================
  Files         199      199              
  Lines       13741    13741              
==========================================
- Hits        10456     9649     -807     
- Misses       2674     3532     +858     
+ Partials      611      560      -51     

@spencerschrock spencerschrock temporarily deployed to gitlab October 27, 2023 20:12 — with GitHub Actions Inactive
@spencerschrock spencerschrock temporarily deployed to integration-test October 27, 2023 20:12 — with GitHub Actions Inactive
@spencerschrock spencerschrock requested a review from a team as a code owner October 27, 2023 21:53
@spencerschrock spencerschrock temporarily deployed to gitlab October 27, 2023 21:53 — with GitHub Actions Inactive
@spencerschrock spencerschrock temporarily deployed to integration-test October 27, 2023 21:53 — with GitHub Actions Inactive
@spencerschrock spencerschrock merged commit 5f3a0e2 into ossf:main Oct 27, 2023
38 checks passed
@spencerschrock spencerschrock deleted the lint/preset-test branch October 27, 2023 22:05
diogoteles08 pushed a commit to diogoteles08/scorecard that referenced this pull request Nov 13, 2023
* enable test preset

Leaves some opinionated linters disabled with reasons.

Signed-off-by: Spencer Schrock <[email protected]>

* fix tparallel issues.

Signed-off-by: Spencer Schrock <[email protected]>

---------

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Diogo Teles Sant'Anna <[email protected]>
ashearin pushed a commit to kgangerlm/scorecard-gitlab that referenced this pull request Nov 13, 2023
* enable test preset

Leaves some opinionated linters disabled with reasons.

Signed-off-by: Spencer Schrock <[email protected]>

* fix tparallel issues.

Signed-off-by: Spencer Schrock <[email protected]>

---------

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants