Skip to content

Conversation

@silverwind
Copy link
Member

@silverwind silverwind commented Oct 31, 2025

https://github.com/golangci/golangci-lint/releases/tag/v2.6.0

  • modernize linter is enabled, this is the same as gopls modernize
  • perfsprint linter is disabled because it conflicts with modernize (maybe there is a middle ground)
  • gocritic deprecatedComment is disabled as it conflicts with go-swagger
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 31, 2025
@github-actions github-actions bot added topic/code-linting modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/internal labels Oct 31, 2025
@silverwind
Copy link
Member Author

silverwind commented Oct 31, 2025

I think I will revert these string builder changes and disable the offending rule, it seems useless.

@silverwind
Copy link
Member Author

silverwind commented Oct 31, 2025

The stringsbuilder rule is from modernize actually:

https://github.com/golang/tools/blob/ed067b1f257070c651750130ce5081e4afc1915f/gopls/doc/analyzers.md#stringsbuilder-replace--with-stringsbuilder

It says "This avoids quadratic memory allocation and improves performance." so maybe it is worth it.

I will fix those var names.

@silverwind silverwind marked this pull request as draft October 31, 2025 09:39
@silverwind silverwind marked this pull request as ready for review November 1, 2025 10:46
@silverwind
Copy link
Member Author

I manually cleaned up the stringsbuilder cases, should be ready.

}
text += fmt.Sprintf("[%s](%s) %s", commit.ID[:7], commit.URL,
strings.TrimRight(commit.Message, "\r\n")) + authorName
textSb.WriteString(fmt.Sprintf("[%s](%s) %s", commit.ID[:7], commit.URL,
Copy link
Member

Choose a reason for hiding this comment

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

fmt.Fprintf should be better.

message = fmt.Sprintf("%.47s...", message)
}
text += fmt.Sprintf("[%s](%s) %s - %s", commit.ID[:7], commit.URL, message, commit.Author.Name)
textSb.WriteString(fmt.Sprintf("[%s](%s) %s - %s", commit.ID[:7], commit.URL, message, commit.Author.Name))
Copy link
Member

Choose a reason for hiding this comment

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

As above


text := fmt.Sprintf("[%s:%s] %s\r\n", p.Repo.FullName, branchName, commitDesc)
var textSb strings.Builder
textSb.WriteString(fmt.Sprintf("[%s:%s] %s\r\n", p.Repo.FullName, branchName, commitDesc))
Copy link
Member

Choose a reason for hiding this comment

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

As above

}
text += fmt.Sprintf("[%s](%s) %s", commit.ID[:7], commit.URL,
strings.TrimRight(commit.Message, "\r\n")) + authorName
textSb.WriteString(fmt.Sprintf("[%s](%s) %s", commit.ID[:7], commit.URL,
Copy link
Member

Choose a reason for hiding this comment

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

As above

text := fmt.Sprintf("[%s] %s pushed %s to %s:<br>", repoLink, p.Pusher.UserName, commitDesc, branchLink)

var textSb strings.Builder
textSb.WriteString(fmt.Sprintf("[%s] %s pushed %s to %s:<br>", repoLink, p.Pusher.UserName, commitDesc, branchLink))
Copy link
Member

Choose a reason for hiding this comment

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

As above

// for each commit, generate a new line text
for i, commit := range p.Commits {
text += fmt.Sprintf("%s: %s - %s", htmlLinkFormatter(commit.URL, commit.ID[:7]), commit.Message, commit.Author.Name)
textSb.WriteString(fmt.Sprintf("%s: %s - %s", htmlLinkFormatter(commit.URL, commit.ID[:7]), commit.Message, commit.Author.Name))
Copy link
Member

Choose a reason for hiding this comment

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

As above

for i, commit := range p.Commits {
text += fmt.Sprintf("[%s](%s) %s - %s", commit.ID[:7], commit.URL,
strings.TrimRight(commit.Message, "\r\n"), commit.Author.Name)
textSb.WriteString(fmt.Sprintf("[%s](%s) %s - %s", commit.ID[:7], commit.URL,
Copy link
Member

Choose a reason for hiding this comment

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

As above

var attachmentTextSb strings.Builder
for i, commit := range p.Commits {
attachmentText += fmt.Sprintf("%s: %s - %s", SlackLinkFormatter(commit.URL, commit.ID[:7]), SlackShortTextFormatter(commit.Message), SlackTextFormatter(commit.Author.Name))
attachmentTextSb.WriteString(fmt.Sprintf("%s: %s - %s", SlackLinkFormatter(commit.URL, commit.ID[:7]), SlackShortTextFormatter(commit.Message), SlackTextFormatter(commit.Author.Name)))
Copy link
Member

Choose a reason for hiding this comment

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

As above

var htmlCommitsSb strings.Builder
for _, commit := range p.Commits {
htmlCommits += fmt.Sprintf("\n[%s] %s", htmlLinkFormatter(commit.URL, commit.ID[:7]), html.EscapeString(strings.TrimRight(commit.Message, "\r\n")))
htmlCommitsSb.WriteString(fmt.Sprintf("\n[%s] %s", htmlLinkFormatter(commit.URL, commit.ID[:7]), html.EscapeString(strings.TrimRight(commit.Message, "\r\n"))))
Copy link
Member

Choose a reason for hiding this comment

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

As above

message := strings.ReplaceAll(commit.Message, "\n\n", "\r\n")
text += fmt.Sprintf(" > [%s](%s) \r\n ><font color=\"info\">%s</font> \n ><font color=\"warning\">%s</font>", commit.ID[:7], commit.URL,
message, authorName)
textSb.WriteString(fmt.Sprintf(" > [%s](%s) \r\n ><font color=\"info\">%s</font> \n ><font color=\"warning\">%s</font>", commit.ID[:7], commit.URL,
Copy link
Member

Choose a reason for hiding this comment

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

As above

func (p *Permission) LogString() string {
format := "<Permission AccessMode=%s, %d Units, %d UnitsMode(s): ["
var formatSb strings.Builder
formatSb.WriteString("<Permission AccessMode=%s, %d Units, %d UnitsMode(s): [")
Copy link
Member

Choose a reason for hiding this comment

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

Just use fmt.Fprintf so that no args is needed.

@wxiaoguang
Copy link
Contributor

I really dislike unnecessary lint rules.

And I have strong objection for this Sprintf change, it doesn't improve readability.

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

Labels

lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/internal topic/code-linting

4 participants