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

chore: Linter rule for properly formatted api errors #2123

Merged
merged 7 commits into from Jun 7, 2022

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jun 7, 2022

All Message fields must be proper sentences.

@Emyrk Emyrk marked this pull request as ready for review June 7, 2022 14:06
@Emyrk Emyrk requested a review from kylecarbs June 7, 2022 14:16
Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

This is awesome! One minor nit that I noticed, detail doesn't currently contain omitempty. Is that intended?

@@ -123,6 +123,7 @@ func New(options *Options) *API {
)
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
httpapi.Write(w, http.StatusOK, httpapi.Response{
//nolint:gocritic
Copy link
Member

Choose a reason for hiding this comment

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

That's a good linter 🕵️🕵️🕵️

Copy link
Member Author

Choose a reason for hiding this comment

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

The only message I needed to make an exception for 😄

scripts/rules.go Outdated Show resolved Hide resolved
@Emyrk
Copy link
Member Author

Emyrk commented Jun 7, 2022

httpapi.Response

not a bad idea to add it. Adding it now 👍

@Emyrk Emyrk enabled auto-merge (squash) June 7, 2022 14:29
@Emyrk Emyrk merged commit af401e3 into main Jun 7, 2022
@Emyrk Emyrk deleted the stevenmasley/httapi_linter branch June 7, 2022 14:33
kylecarbs pushed a commit that referenced this pull request Jun 10, 2022
* chore: Linter rule for properly formatted api errors
* Add omitempty to 'Detail' field
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.

None yet

2 participants