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

lint: update linter libraries #34015

Closed
wants to merge 1 commit into from

Conversation

RaduBerinde
Copy link
Member

This change updates honnef.co/go/tools and errcheck. We were
running into problems with the new releases with go 1.10, but they
work fine with go 1.11.

Our lint code had to be changed to work the new release of
honnef.co/go/tools.

This change also includes some code fixes for lint issues that are
detected only by the new versions.

Changes:

Informs #30774.

Release note: None

This change updates `honnef.co/go/tools` and `errcheck`. We were
running into problems with the new releases with go 1.10, but they
work fine with go 1.11.

Our lint code had to be changed to work the new release of
`honnef.co/go/tools`.

This change also includes some code fixes for lint issues that are
detected only by the new versions.

Changes:
 - kisielk/errcheck@55d8f50...e14f8d5
 - dominikh/go-tools@d73ab98...c2f93a9
 - new indirect dependency https://github.com/BurntSushi/toml

Release note: None
@RaduBerinde RaduBerinde requested review from tbg and a team January 15, 2019 14:52
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@RaduBerinde
Copy link
Member Author

The CI build is behaving as if it's running go 1.10..

@RaduBerinde
Copy link
Member Author

Never mind, it is running 1.11. Not sure what the difference is, locally all the lint tests pass in ~3 minutes.

@RaduBerinde
Copy link
Member Author

TestUnused takes close to 16GB of RAM. It used to be closer to 8GB before. The builders have 16GB so it's probably swapping and timing out.

Not sure what to do. We could increase the builders RAM, but I don't like making things slower and use more memory just in the name of using newer bits. Maybe I should file an issue to investigate why it's using so much more.

@RaduBerinde
Copy link
Member Author

I filed dominikh/go-tools#394.

@RaduBerinde
Copy link
Member Author

I will pull the code fixes in a separate PR, as this will probably won't merge any time soon.

@RaduBerinde
Copy link
Member Author

Closing this for now; we will stay with the old version for now given the unacceptable memory usage increase with the new version.

Copied the diff into a gist for potential reuse later, just in case I accidentally nuke the branch: https://gist.github.com/RaduBerinde/3f8c7f7cc1b35bacd8791b4635d49a48

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