Skip to content

Conversation

@ishanarya0
Copy link
Member

No description provided.

@ishanarya0
Copy link
Member Author

ishanarya0 commented May 17, 2022

Added golangci linters.
Should we use the following linters as well?

@ishanarya0
Copy link
Member Author

[ISSUE] Golangci Github Action is unable to identify some linter(s) hence these failing checks.

@spy16
Copy link
Contributor

spy16 commented May 18, 2022

I just ticked the ones i think we can use.

  • gofumpt - this one would rewrite go files, so may be we could just put it as part of makefile. That way, we always commit properly formatted files.
  • nlreturn - i think it's arguable wether this is a good idea or not. Leaving an empty line before every return isn't a good idea in my opinion. I usually prefer to keep relevant things grouped together as much as possible.
  • exhauststruct - i don't think it's a good approach to enforce this strictly. Zero-values are in many cases very useful defaults and even recommended by the community. As such, it might be preferable to allow users to init only specific fields.

@ishanarya0 ishanarya0 force-pushed the linters branch 3 times, most recently from c69efec to fc8d3e4 Compare May 18, 2022 08:15
@ishanarya0 ishanarya0 force-pushed the linters branch 2 times, most recently from c7ee54d to e4084c7 Compare May 18, 2022 08:20
@ishanarya0
Copy link
Member Author

I just ticked the ones i think we can use.

  • gofumpt - this one would rewrite go files, so may be we could just put it as part of makefile. That way, we always commit properly formatted files.
  • nlreturn - i think it's arguable wether this is a good idea or not. Leaving an empty line before every return isn't a good idea in my opinion. I usually prefer to keep relevant things grouped together as much as possible.
  • exhauststruct - i don't think it's a good approach to enforce this strictly. Zero-values are in many cases very useful defaults and even recommended by the community. As such, it might be preferable to allow users to init only specific fields.
  • gofumpt: yes let's add it to make file
  • nlreturn: yes, I was unsure about that practice. REMOVING it.
  • exhauststrcut: REMOVING it as well.

@ishanarya0 ishanarya0 force-pushed the linters branch 3 times, most recently from f3e9125 to e0437cf Compare May 24, 2022 11:01
@rohilsurana rohilsurana merged commit 09b5b1c into main May 25, 2022
@rohilsurana rohilsurana deleted the linters branch May 25, 2022 04:58
@ravisuhag ravisuhag linked an issue May 26, 2022 that may be closed by this pull request
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.

[CI] Update project linters

5 participants