Go linter errors? · Issue #741 · github/gh-ost · GitHub
Skip to content

Go linter errors? #741

Closed
Closed

Description

@zmoazeni

Hiya! One of the nice features of Visual Studio Code is that it integrates linters very easily.

Unfortunately, checking a few different linters comes up with a slew of issues in this codebase. I've moved to golangci-lint because it's one of the few that supports Go modules.

Here's a snippet of the issues I'm seeing:

2019-05-17_13-31

Some questions I have:

  1. Should we resolve these? If so, what would be the best way to PR these changes?
  2. Should we add the linter(s) to CI to prevent getting into the same scenario in the future?

Activity

  1. shlomi-noach commented on May 29, 2019

    @shlomi-noach
    Contributor

    Hi @zmoazeni! Sorry for the late reply.

    So, I'm ambivalent. Some lint errors are excellent advice. Some are general programming practices (and I understand they can encourage a single, unified practice). Some I plainly disagree with (cyclomatic complexity of this function?). Some are nitpicks for upper/lower case letters (HTTP or Http?). Some are about method signatures.

    I confess that I'm not a purist and I do not adhere to each and every golang common practice. For this reason, I'm not going to fix all lint errors/comments. I'm OK to fix some of these, but I don't expect linter to run clean on gh-ost. So we will always be left with some warnings.

    What do you think?

  2. zmoazeni commented on May 29, 2019

    @zmoazeni
    ContributorAuthor

    Sorry for the late reply.

    Don't be sorry at all. I get it.

    I get where you're coming from. It certainly falls down a rabbit hole of tweaking stuff just to tweak stuff with no real gains.

    Speaking to the variable/package casing, I do think some of the nitpicky stuff helps others that aren't versed in go to feel confident they're "speaking go" instead of coming at it with a different language's "accent", if that makes sense. A linter helps keep them on track. I've noticed that with rubocop and newer Ruby devs.

    One of my personal nitpicks is the required comment on all public functions/structures. They end up with unhelpful comments that looks like:

    // MyFunc takes an integer and returns a string
    func MyFunc(x int) string {
      // ...
    }

    I personally would rather that comment not exist at all.


    For what it's worth I've seen some good use with https://github.com/golangci/golangci-lint it might check the boxes you're looking for:

    1. With a .golangci.yml you can pick and choose which rules you want to enable/disable.
    2. In the code you can use //nolint[:linter1,linter2,...] to opt out of rules that you typically want to follow, except in certain cases.
    3. You can use it to enable other linters. Such as https://go-critic.github.io/overview.html and https://github.com/kyoh86/scopelint which can watch out for serious gotchas.

    My suggestion would be to use a linter but be clear about what things you care about what you don't. Even if it disables everything by default and only enables a handful of useful linters. That said, if you would rather not go down that path, I would respect your decision.

  3. shlomi-noach commented on May 29, 2019

    @shlomi-noach
    Contributor

    My suggestion would be to use a linter but be clear about what things you care about what you don't. Even if it disables everything by default and only enables a handful of useful linters.

    OK, I'm game to give this a shot!

  4. timvaillancourt commented on Jul 18, 2022

    @timvaillancourt
    Collaborator

    👋 closed this one as we now have golang-ci automation for PRs and a script/lint for running it locally. See #1127, #1145, #1131, #1132 and #1149 for more

    Feel free to open a new issue re: linting! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions