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

ci: address multiple lint rules #2869

Merged
merged 3 commits into from
Feb 19, 2024
Merged

Conversation

nickajacks1
Copy link
Member

Description

Disable tagalign.
Tagalign requires awkward manual formatting and doesn't provide much value for readability.

Enable mirror.
Mirror warns against certain cases of useless conversion between string and []byte.

Enable perfsprint.
This linter encourages replacing several functions from the fmt package with faster alternatives. While fixing issues, I added a few exported error types rather than returning a naked errors.New().

Type of Change

  • Code consistency (non-breaking change which improves code reliability and robustness)

Checklist

Before you submit your pull request, please make sure you meet these requirements:

  • Conducted a self-review of the code and provided comments for complex or critical parts.
  • Ensured that new and existing unit tests pass locally with the changes.
  • Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.

Tagalign requires awkward manual formatting and doesn't provide much
value for readability.
mirror warns against certain cases of useless conversion between string
and []byte.
This linter encourages replacing several functions from the fmt package
with faster alternatives. While fixing issues, I also added a few
exported error types rather than returning a naked errors.New().
@nickajacks1 nickajacks1 requested a review from a team as a code owner February 19, 2024 04:05
@nickajacks1 nickajacks1 requested review from gaby, sixcolors, ReneWerner87 and efectn and removed request for a team February 19, 2024 04:05
@nickajacks1
Copy link
Member Author

Here's an example of what tagalign will enforce:

        type Request struct {
                QueryParam  string `query:"query_param"`
                HeaderParam string `header:"header_param"`
                BodyParam   string `form:"body_param"     json:"body_param" xml:"body_param"`
        }

^ it will complain if you don't have this exact number of spaces between the form and json tags for BodyParam.
It might increase readability in other scenarios, but it seemed more annoying than helpful.
Please feel free to push back if you disagree me here.

Copy link
Member

@sixcolors sixcolors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, go fmt won’t automatically align the tags, which makes it an annoying manual task. LGTM!

Copy link
Member

@gaby gaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, i'm fine with disabling tagalign

@ReneWerner87 ReneWerner87 merged commit 4c68e02 into gofiber:main Feb 19, 2024
17 checks passed
@nickajacks1 nickajacks1 deleted the lint-misc branch February 19, 2024 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants