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

Extend linting #384

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Extend linting #384

wants to merge 3 commits into from

Conversation

monstermunchkin
Copy link
Contributor

No description provided.

@srieckhof
Copy link
Contributor

I'll wait with reviewing this PR until bun and sentry are completely finished.

Comment on lines +392 to +405
case "array":
name, ok := data.Value.Items.Value.Properties["type"].Value.Enum[0].(string)
if !ok {
return nil, fmt.Errorf("expected as string got %T", data.Value.Items.Value.Properties["type"].Value.Enum[0])
}

rel.Index().Op("*").Id(goNameHelper(name)).Tag(tags)
// case object = belongs-to
case "object": // nolint: goconst
name := data.Value.Properties["type"].Value.Enum[0].(string)
case "object":
name, ok := data.Value.Properties["type"].Value.Enum[0].(string)
if !ok {
return nil, fmt.Errorf("expected as string got %T", data.Value.Properties["type"].Value.Enum[0])
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make these two error messages distinguishable, that is, depending on the case?

repliesJSONAPI := e.Header().Get("Content-Type") == runtime.JSONAPIContentType
requestsJSONAPI := e.req.Header.Get("Accept") == runtime.JSONAPIContentType

if e.statusCode >= 400 && requestsJSONAPI && !repliesJSONAPI {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this 400 be replaced by http.BadRequest (or something like this) as well?

Comment on lines 85 to 87
return fmt.Errorf(
"The %s member of the links object was not a string or link object",
"the %s member of the links object was not a string or link object",
k,
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps just on one line and use %q instead of %s.

@@ -489,7 +496,7 @@ func handleTime(attribute interface{}, args []string, fieldValue reflect.Value)
if isIso8601 {
var tm string
if v.Kind() == reflect.String {
tm = v.Interface().(string)
tm, _ = v.Interface().(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you (here and below) suppress error handling on purpose?

@@ -21,7 +21,7 @@ func Hide(ctx context.Context, err, exposedErr error) error {

ret := err
if exposedErr != nil {
ret = fmt.Errorf("%w: %s", exposedErr, err)
ret = fmt.Errorf("%w: %s", exposedErr, err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ret = fmt.Errorf("%w: %s", exposedErr, err.Error())
ret = fmt.Errorf("%w: %w", exposedErr, err)

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a mixture of verbs (%s, %w) I don't really understand - already in the original. Do you know why? How about just using %w?

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

Successfully merging this pull request may close these issues.

2 participants