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

deps: github.com/golangci/golangci-lint/cmd/golangci-lint@v1.16.0 #8143

Merged
merged 2 commits into from
Apr 2, 2019

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Apr 1, 2019

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

References:

We can try using the GOGC flag to influence garbage collection if necessary.

Updated via:

$ go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.16.0
$ go mod tidy
$ go mod vendor

References:
* https://github.com/golangci/golangci-lint/releases/tag/v1.16.0
* #7993 (comment)

Updated via:

```console
$ go get github.com/golangci/golangci-lint/cmd/golangci-lint@v1.16.0
$ go mod tidy
$ go mod vendor
```
@bflad bflad added dependencies Used to indicate dependency changes. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. provider Pertains to the provider itself, rather than any interaction with AWS. labels Apr 1, 2019
@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Apr 1, 2019
@bflad bflad requested a review from a team April 1, 2019 12:29
@bflad
Copy link
Contributor Author

bflad commented Apr 1, 2019

RSS from v1.15.0 to v1.16.0 is only a little worse (better than the problematic commit in the middle) so we might be okay to skip tuning GOGC for now. If it has memory trouble in the future we can revisit the customization.

# golangci-lint v1.16.0
5605507072  maximum resident set size
# golangci-lint from commit at time of https://github.com/terraform-providers/terraform-provider-aws/pull/7993#discussion_r270681029
6227824640  maximum resident set size
# golangci-lint v1.15.0
5142007808  maximum resident set size

@bflad
Copy link
Contributor Author

bflad commented Apr 1, 2019

The above is without enabling staticcheck. Once its enabled, we do need to start looking at tuning GOGC, e.g.

$ time -l ~/go/bin/golangci-lint run ./aws
       26.44 real        89.50 user         8.99 sys
6343839744  maximum resident set size
...

$ GOGC=50 time -l ~/go/bin/golangci-lint run ./aws
       30.10 real       122.25 user         9.22 sys
6118998016  maximum resident set size
...

$ GOGC=30 time -l ~/go/bin/golangci-lint run ./aws
       33.93 real       161.87 user         7.80 sys
5039095808  maximum resident set size

I'm adding a second commit which enables staticcheck and tunes GOGC=30 but we can discuss or remove as necessary.

@bflad bflad added this to the v2.5.0 milestone Apr 2, 2019
@bflad bflad merged commit 1ba231d into master Apr 2, 2019
@bflad bflad deleted the v-golangci-lint-v1.16.0 branch April 2, 2019 13:12
@ghost
Copy link

ghost commented Mar 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
@breathingdust breathingdust added the linter Pertains to changes to or issues with the various linters. label Oct 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Used to indicate dependency changes. linter Pertains to changes to or issues with the various linters. provider Pertains to the provider itself, rather than any interaction with AWS. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants