Skip to content

Conversation

@yegor256
Copy link
Member

closes #113

@yegor256
Copy link
Member Author

@volodya-lombrozo please, check

Copy link
Collaborator

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@yegor256 It looks fine to me when we use capital letters in logs, but it's a common practice to use lower-case in error messages. It's an idiomatic way, let's say.

data, err := json.Marshal(body)
if err != nil {
return "", fmt.Errorf("error marshaling request body: %w", err)
return "", fmt.Errorf("Error marshaling request body: %w", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@yegor256

Error strings should not be capitalized

From: https://go.dev/wiki/CodeReviewComments#error-strings

@@ -1,3 +1,4 @@
// Package facilitator is for the facilitator.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@yegor256 Do we need these meaningless comments?

Copy link
Member Author

Choose a reason for hiding this comment

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

@volodya-lombrozo golangci-lint demands us to have them. I don't know how did it work without them.

Copy link
Collaborator

@volodya-lombrozo volodya-lombrozo Sep 17, 2025

Choose a reason for hiding this comment

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

@yegor256 .golangci.yml

     staticcheck:
       checks: ["all", "-ST1005", "-ST1000"]

@yegor256
Copy link
Member Author

@volodya-lombrozo I reverted the changes, please look again

Copy link
Collaborator

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@yegor256 Looks good to me. Thanks!

@volodya-lombrozo volodya-lombrozo merged commit aa71962 into master Sep 17, 2025
6 checks passed
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.

some user-facing messages are not consistently formatted

3 participants