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

Fix inter issues in master branch #769

Closed
18 tasks done
na-- opened this issue Sep 14, 2018 · 1 comment
Closed
18 tasks done

Fix inter issues in master branch #769

na-- opened this issue Sep 14, 2018 · 1 comment

Comments

@na--
Copy link
Member

na-- commented Sep 14, 2018

What?

We should fix the linter issues that exist in our master branch.

Tasks

  1. maintenance
    codebien
  2. breaking change maintenance refactor
    olegbespalov
  3. maintenance refactor
    mstoykov
  4. lower prio maintenance refactor
    olegbespalov
  5. lower prio maintenance refactor
    olegbespalov
  6. maintenance
    mstoykov
  7. maintenance
    olegbespalov
  8. lower prio maintenance refactor
    olegbespalov
  9. maintenance
    olegbespalov
  10. bug maintenance
  11. maintenance
    mstoykov
  12. maintenance
    mstoykov

Right now we use the golangci-lint as the main tool for linting. The latest report:

golangci-lint run --out-format=tab | awk 'NF { print $1 }' | sed -E 's|/[^/]*(:[0-9]+){0,2}$||' | uniq -c | sort -rn
 233 js/modules/k6/html
    112 js/modules/k6/http
     48 lib
     38 metrics
     32 lib/executor
     28 lib/netext/httpext
     26 lib/netext
     22 cloudapi
      5 lib/fsext
      5 lib
      3 output/statsd
      3 output/json
      3 js/modules/k6/grpc
      2 lib/types
      2 lib/netext/grpcext
      2 lib/consts
      2 js/modules/k6/metrics
      2 js/common
      2 cmd

Why?

Most of those are about undocumented methods or using underscores or ALL_CAPS in variables, but it's still worth it to spend some time and eliminate, or at least substantially reduce, the number of linter complaints.

Fixing linter issues would significantly help any potential contributors because:

  • it forces best practices
  • it would force us to improve the in-code documentation, tremendously helping new people who are unfamiliar with the codebase
  • it would make the code more idiomatic
  • a lot of Go editors have golint/golangci-lint automatically enabled, and when you open some files with them, there are dozens of issues reported
  • we can improve our Go report card score, after all we've added it in the top of the README...
  • and many more
@na--
Copy link
Member Author

na-- commented Jul 19, 2019

This tool might be helpful for some of the drudge work: https://github.com/cuonglm/gocmt

@mstoykov mstoykov mentioned this issue Oct 13, 2023
5 tasks
@olegbespalov olegbespalov changed the title Fix golint issues Fix inter issues in master branch Nov 21, 2023
This was referenced Dec 1, 2023
@mstoykov mstoykov mentioned this issue Feb 1, 2024
5 tasks
mstoykov added a commit that referenced this issue Mar 11, 2024
Some leftover lint fixes and just move to crypto/rand for the idempotency
token.

Part of #769
mstoykov added a commit that referenced this issue Apr 4, 2024
Some leftover lint fixes and just move to crypto/rand for the idempotency
token.

Part of #769
@mstoykov mstoykov mentioned this issue Apr 22, 2024
5 tasks
mstoykov added a commit that referenced this issue Apr 25, 2024
As now all lint issues are fixed we can enable without that option.

Unfortunately the option only looks at code at and around the places
that was changed and if any of it has new lint errors it will trigger.

That does mean that if you update golangci-lint - it won't showcase new
lint errors it triggers.

Also won't work if you add a bunch of code to the end an already
somewhat long function. As the lint error will be at the beginning of
the function.

Closes #769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant