Skip to content

Conversation

@electricface
Copy link
Member

@electricface electricface commented Jan 29, 2026

  • Replace fmt.Errorf with errors.New in multiple files for
    error returns
  • Fix incorrect string concatenation in fmt.Errorf within
    service_trigger/service.go; use proper formatting parameters instead

fix: 修复go 1.24.6 版本编译失败问题

  • 将多个文件中错误返回由 fmt.Errorf 改为 errors.New
  • 修正 service_trigger/service.go 中 fmt.Errorf
    格式化字符串拼接错误,改为正确的格式化参数传入

Log: 修复go 1.24.6 版本编译失败问题
Influence: 无

Summary by Sourcery

Fix error construction and formatting to resolve compilation issues with newer Go versions.

Bug Fixes:

  • Replace fmt.Errorf with errors.New for simple error values to satisfy updated Go compiler constraints.
  • Correct incorrect fmt.Errorf usage in service_trigger/service.go by using a formatting verb instead of string concatenation.

- Replace `fmt.Errorf` with `errors.New` in multiple files for
error returns
- Fix incorrect string concatenation in `fmt.Errorf` within
service_trigger/service.go; use proper formatting parameters instead

---

fix: 修复go 1.24.6 版本编译失败问题

- 将多个文件中错误返回由 fmt.Errorf 改为 errors.New
- 修正 service_trigger/service.go 中 fmt.Errorf
格式化字符串拼接错误,改为正确的格式化参数传入

Log: 修复go 1.24.6 版本编译失败问题
Influence: 无
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 29, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR fixes Go 1.24.6 build issues by replacing inappropriate uses of fmt.Errorf with errors.New where no formatting is needed, and by correcting a misconstructed fmt.Errorf call in the service trigger logic.

Flow diagram for doAction command execution and error handling

flowchart TD
    A[doAction cmd] --> B[exec.Command /bin/sh -c cmd]
    B --> C[CombinedOutput]
    C --> D{err != nil}
    D -- yes --> E[Create error with errors.New using command output]
    E --> F[Return error]
    D -- no --> G[Return nil]
Loading

Flow diagram for Service.check monitor type validation

flowchart TD
    A[Service.check] --> B{service.Monitor.Type != DBus}
    B -- yes --> C[Return error using fmt.Errorf with format string and Monitor.Type]
    B -- no --> D{service.Monitor.Type == DBus}
    D -- yes --> E[Continue DBus-specific checks]
    D -- no --> F[End]
Loading

File-Level Changes

Change Details Files
Replace non-formatting uses of fmt.Errorf with errors.New to avoid unnecessary formatting behavior and potential Go 1.24.6 build or vet issues.
  • Change error construction from fmt.Errorf(...) to errors.New(...) where the argument is already a plain string.
  • Preserve existing error messages while simplifying error creation and dependencies on fmt.
accounts1/utils.go
bin/user-config/config_datas.go
inputdevices1/utils.go
Fix incorrect fmt.Errorf usage in service trigger monitoring type validation.
  • Correct string formatting by using a format string with %q and passing Monitor.Type as a separate argument instead of concatenating into the format string.
  • Ensure the resulting error message is well-formed and compatible with Go 1.24.6 compilers and linters.
service_trigger/service.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码diff主要涉及了Go语言中错误处理方式的调整以及字符串格式化的问题。以下是对每个文件的详细审查和改进建议:

1. accounts1/utils.go, bin/user-config/config_datas.go, inputdevices1/utils.go

修改内容
fmt.Errorf(string(out))fmt.Errorf(ErrCodeAuthFailed.String()) 修改为 errors.New(string(out))errors.New(ErrCodeAuthFailed.String())

审查意见

  • 语法逻辑:修改是正确的。errors.New 是创建基础错误对象的标准方式。之前的代码使用 fmt.Errorf 但没有提供格式化动词(如 %s),虽然 fmt.Errorf 可以接受单个字符串参数并返回错误,但在不进行格式化操作时,直接使用 errors.New 语义更清晰,开销更小。
  • 代码质量:改进了代码的语义明确性。当不需要格式化字符串时,使用 errors.New 是更好的实践。
  • 代码性能errors.Newfmt.Errorf 更轻量级,因为它不需要解析格式化字符串,减少了不必要的函数调用开销。
  • 代码安全:无直接影响。

改进建议
bin/user-config/config_datas.goinputdevices1/utils.godoAction 函数中,exec.Command 的输出 out 可能包含敏感信息(如命令执行时的上下文),直接将其作为错误信息抛出可能存在信息泄露风险。建议对错误信息进行脱敏处理,或者仅返回通用的错误描述,将详细输出记录到日志中。

改进代码示例

// 建议改进 doAction 函数
func doAction(cmd string) error {
    out, err := exec.Command("/bin/sh", "-c", cmd).CombinedOutput()
    if err != nil {
        // 记录详细日志
        log.Printf("command failed: %s, output: %s", cmd, string(out))
        // 返回给调用者的错误信息可以更模糊一些,避免泄露系统细节
        return errors.New("failed to execute command")
    }
    return nil
}

2. service_trigger/service.go

修改内容
return fmt.Errorf("unknown Monitor.Type %q" + service.Monitor.Type) 修改为 return fmt.Errorf("unknown Monitor.Type %q", service.Monitor.Type)

审查意见

  • 语法逻辑:修改非常正确且必要。原代码使用了字符串拼接 (+) 而不是 fmt.Errorf 的参数列表。原代码实际上是将 %q 当作普通字符处理,然后拼接了 service.Monitor.Type,导致错误信息格式不正确(例如输出 unknown Monitor.Type %qSomeType 而不是 unknown Monitor.Type "SomeType")。修改后使用了正确的格式化参数,逻辑修复。
  • 代码质量:修复了潜在的Bug,提高了代码的正确性。
  • 代码性能fmt.Errorf 使用参数列表通常比字符串拼接更高效,且可读性更好。
  • 代码安全:无直接影响。

改进建议

  1. 逻辑冗余:在 check 函数中,第一个 if 判断 Type != "DBus",紧接着又判断 Type == "DBus"。逻辑上,如果第一个条件成立,函数已经返回了。第二个 if 判断是多余的。如果后续代码只针对 DBus 类型,可以直接去掉这个 if 判断,或者将后续逻辑放在 else 块中。
  2. 错误类型:如果这是一个库代码,建议定义具体的错误变量(如 ErrUnknownMonitorType),使用 errors.Newfmt.Errorf 包装它,这样调用者可以使用 errors.Is 进行判断。

改进代码示例

func (service *Service) check() error {
    if service.Monitor.Type != "DBus" {
        // 使用预定义的错误或更清晰的错误信息
        return fmt.Errorf("unsupported monitor type: %q", service.Monitor.Type)
    }

    // 既然上面已经确认了 Type 是 "DBus",这里不需要再判断
    // 后续针对 DBus 的逻辑...
    
    return nil
}

总结

这次代码提交主要修复了 service_trigger/service.go 中的格式化字符串错误,并在其他文件中将 fmt.Errorf 规范化为 errors.New,这些都是积极的改进。建议在后续开发中注意命令执行错误的信息泄露风险,以及简化冗余的条件判断逻辑。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In both doAction helpers, consider including the original err from exec.Command (e.g., via fmt.Errorf("%s: %w", out, err)) so callers get both the command output and the underlying failure cause instead of losing that context.
  • In Service.check, since the Monitor.Type != "DBus" branch already returns, the subsequent if service.Monitor.Type == "DBus" can be simplified to an else for slightly clearer control flow.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In both `doAction` helpers, consider including the original `err` from `exec.Command` (e.g., via `fmt.Errorf("%s: %w", out, err)`) so callers get both the command output and the underlying failure cause instead of losing that context.
- In `Service.check`, since the `Monitor.Type != "DBus"` branch already returns, the subsequent `if service.Monitor.Type == "DBus"` can be simplified to an `else` for slightly clearer control flow.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@fly602 fly602 merged commit 8a6a977 into linuxdeepin:master Jan 29, 2026
14 of 17 checks passed
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface, fly602

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@electricface electricface deleted the swt/fix-build branch January 29, 2026 09:23
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.

3 participants