Skip to content

fix: Solve the issue where "lastore-apt-clean" outputs warning messages#249

Merged
qiuzhiqian merged 1 commit into
masterfrom
opt_lastore_apt_clean
Oct 31, 2025
Merged

fix: Solve the issue where "lastore-apt-clean" outputs warning messages#249
qiuzhiqian merged 1 commit into
masterfrom
opt_lastore_apt_clean

Conversation

@Fire-dtx
Copy link
Copy Markdown
Contributor

Description: A file is detected missing during the initialization of the config

Log: Use the dde-dconfig command to retrieve configuration values

Bug: https://pms.uniontech.com/bug-view-338213.html

Influence: Command output

Description: A file is detected missing during the initialization of the config

Log: Use the dde-dconfig command to retrieve configuration values

Bug: https://pms.uniontech.com/bug-view-338213.html

Influence: Command output
@Fire-dtx Fire-dtx force-pushed the opt_lastore_apt_clean branch from 51279ac to 7baa938 Compare October 31, 2025 03:49
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

我来对这个代码变更进行审查:

  1. 代码逻辑和功能改进:
  • 引入了增量更新(incremental update)的支持,这是一个新功能
  • 将配置检查从命令行参数移到了配置文件读取,这是一个好的改进
  • 移除了未使用的import"path",提高了代码整洁度
  1. 代码质量问题:
  • 在getArchiveInfo()和getNeedCleanCacheSize()函数中存在重复代码,建议提取为公共函数
  • isIncrementalUpdate变量的命名不够清晰,建议改为incrementalUpdateFlag
  • 配置文件路径"/var/lib/lastore/config.json"被硬编码在多处,建议定义为常量
  1. 性能优化建议:
  • 每次调用getArchiveInfo()和getNeedCleanCacheSize()都会重新读取配置文件,建议考虑缓存配置信息
  • 可以将config对象作为参数传递,而不是在每个函数中重新创建
  1. 安全性考虑:
  • 配置文件路径使用了固定的绝对路径,建议增加路径存在性检查
  • 执行外部命令时没有对命令参数进行验证,建议增加参数校验
  1. 具体改进建议:
// 定义常量
const ConfigPath = "/var/lib/lastore/config.json"

// 提取公共函数
func getCleanCommandArgs() []string {
    args := []string{"-print-json"}
    if NewConfig(ConfigPath).IncrementalUpdate {
        args = append(args, "--incremental-update")
    }
    return args
}

// 改进后的函数
func getArchiveInfo() (string, error) {
    out, err := exec.Command("/usr/bin/lastore-apt-clean", getCleanCommandArgs()...).Output()
    if err != nil {
        return "", err
    }
    return string(out), nil
}

func getNeedCleanCacheSize() (float64, error) {
    output, err := exec.Command("/usr/bin/lastore-apt-clean", getCleanCommandArgs()...).Output()
    if err != nil {
        return 0, err
    }
    // 其余处理逻辑...
}
  1. 日志级别改进:
  • 在main.go中,将清理失败的日志从Warning改为Debug是合适的,因为这不是关键错误
  1. 其他建议:
  • 考虑将配置读取和验证逻辑封装到单独的函数中
  • 可以增加配置文件格式验证,确保配置的正确性
  • 建议添加单元测试来验证新的增量更新功能

这些改进可以提高代码的可维护性、可读性和健壮性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Fire-dtx, qiuzhiqian

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

@qiuzhiqian qiuzhiqian merged commit 6a0f921 into master Oct 31, 2025
26 of 28 checks passed
@qiuzhiqian qiuzhiqian deleted the opt_lastore_apt_clean branch October 31, 2025 05:15
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