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

Potential code quality issues found #260

Open
ghost opened this issue Jul 28, 2020 · 3 comments
Open

Potential code quality issues found #260

ghost opened this issue Jul 28, 2020 · 3 comments

Comments

@ghost
Copy link

ghost commented Jul 28, 2020

I forked this repo a while ago and ran a DeepSource analysis on it. DeepSource found a variety of different issues categorized based on their types and severity which you can view here.

Brief description -
Anti-Patterns: 36
Bug Risks: 23
Security Issues: 2

You can find a detailed description and fixes for some of them here.

Please let me know what issues you'd like to focus on fixing and I'd be happy to take a look into it. Also, you can choose to hide certain types of issues too (if you wish to ignore them or you believe it is a false positive). I'd also be happy to send a patch with the DeepSource configuration file with the required fixes.

You can find the required configuration file for DeepSource here.

@TysonAndre
Copy link
Contributor

TysonAndre commented Jul 28, 2020

I've already set up golangci-lint locally and in travis, and prefer to keep using it locally and avoid multiple linter groups. Although I haven't really put much effort into configuring golangci-lint beyond the defaults or testing out the disabled linters, it seems to catch many of the same issues right now with the proper configuration https://golangci-lint.run/usage/configuration/

I'd disabled errcheck because there were too many false positives with byte buffers that shouldn't fail - I guess I could panic in that case.

The style issues seem unimportant to fix - in some cases the style was deliberate if more cases were added to an else branch or switch, though it seems unlikely for many of them.

For sha1, that's fairly low priority because tokens used by others won't be guessable and creating colliding tokens won't really matter for the ways uniqush was used


# TODO: Re-enable errcheck
linters:
    enable:
        - errcheck
        - deadcode
        - gocritic
        - gofmt
        - golint
        - gosec
        - govet
        - ineffassign
        - misspell
        - scopelint
        - structcheck
        - unconvert
        - unparam
        - varcheck
        - vet
        - vetshadow
linters-settings:
    gocritic:
        enabled-tags:
            - experimental

@TysonAndre
Copy link
Contributor

and the Signal(os.Kill) is definitely a no-op for Kill, but I suppressed it with megacheck because there were too many issues at the time to check the docs and it would have no impact if it was a no-op.

@ghost
Copy link
Author

ghost commented Jul 28, 2020

Thank you for the inputs. Is there anything specific that you'd like me to help you out with?

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

No branches or pull requests

1 participant