Skip to content

Issue ee 2354 #3032

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Issue ee 2354 #3032

wants to merge 2 commits into from

Conversation

ColdWaterLW
Copy link
Collaborator

@ColdWaterLW ColdWaterLW commented May 6, 2025

User description

关联的 issue

https://github.com/actiontech/sqle-ee/issues/2354

描述你的变更

确认项(pr提交后操作)

Tip

请在指定复审人之前,确认并完成以下事项,完成后✅


  • 我已完成自测
  • 我已记录完整日志方便进行诊断
  • 我已在关联的issue里补充了实现方案
  • 我已在关联的issue里补充了测试影响面
  • 我已确认了变更的兼容性,如果不兼容则在issue里标记 not_compatible
  • 我已确认了是否要更新文档,如果要更新则在issue里标记 need_update_doc

link actiontech/dms#447


Description

  • 修正通过 git tag 命令获取最新标签逻辑

  • 优化 PROJECT_VERSION 的版本号解析流程

  • 移除 v 前缀确保版本格式正确


Changes walkthrough 📝

Relevant files
Bug fix
Makefile
更新版本标签与解析逻辑                                                                                           

Makefile

  • 将 HEAD_TAG 获取方式由 git describe 改为 git tag --points-at HEAD | tail -n1
  • 更新 PROJECT_VERSION 的构造,依据 HEAD_TAG 判断版本字符串
  • 使用 sed 命令移除 v 前缀,保证版本号格式一致
  • +2/-2     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @actiontech-bot actiontech-bot requested review from LordofAvernus and removed request for LordofAvernus May 6, 2025 09:13
    Copy link

    github-actions bot commented May 6, 2025

    PR Reviewer Guide 🔍

    🎫 Ticket compliance analysis 🔶

    2354 - Partially compliant

    Compliant requirements:

    • 采用 git tag --points-at HEAD 获取最新标签
    • 在 PROJECT_VERSION 中处理标签与分支逻辑

    Non-compliant requirements:

    Requires further human verification:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    标签获取更新

    新增的命令使用 git tag --points-at HEAD | tail -n1 2>/dev/null 来获取 HEAD 的标签,请确认在没有标签的情况下该逻辑能正确处理,避免潜在异常。

    HEAD_TAG = $(shell git tag --points-at HEAD | tail -n1 2>/dev/null)
    版本号解析逻辑

    使用 HEAD_TAG 变量判断后,依据是否存在标签设置 PROJECT_VERSION,请确认该逻辑在各种分支及标签状态下均能保持一致性和正确性。

    PROJECT_VERSION = $(shell if [ -n "$(HEAD_TAG)" ]; then echo $(HEAD_TAG) | sed 's/v\(.*\)/\1/'; else git rev-parse --abbrev-ref HEAD | sed 's/release-\(.*\)/\1/' | tr '-' '\n' | head -n1; fi)

    Copy link

    github-actions bot commented May 6, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    引号包围 tag 字符串

    建议在 echo 时将 $(HEAD_TAG) 用双引号包围,以防 tag 字符串中出现空格或特殊字符,确保 sed 命令能正确执行。这样能更稳健地处理版本号转换。

    Makefile [46]

    -PROJECT_VERSION = $(shell if [ -n "$(HEAD_TAG)" ]; then echo $(HEAD_TAG) | sed 's/v\(.*\)/\1/'; else git rev-parse --abbrev-ref HEAD | sed 's/release-\(.*\)/\1/' | tr '-' '\n' | head -n1; fi)
    +PROJECT_VERSION = $(shell if [ -n "$(HEAD_TAG)" ]; then echo "$(HEAD_TAG)" | sed 's/v\(.*\)/\1/'; else git rev-parse --abbrev-ref HEAD | sed 's/release-\(.*\)/\1/' | tr '-' '\n' | head -n1; fi)
    Suggestion importance[1-10]: 6

    __

    Why: Wrapping $(HEAD_TAG) in double quotes enhances robustness against spaces or special characters. This is a small but useful improvement in handling version strings.

    Low
    对 tag 输出进行排序

    考虑对获取 tag 的命令结果进行排序,确保使用 tail -n1 时始终获得预期的 tag。当前命令可能因多标签存在而导致非确定性结果。

    Makefile [3]

    -HEAD_TAG = $(shell git tag --points-at HEAD | tail -n1 2>/dev/null)
    +HEAD_TAG = $(shell git tag --points-at HEAD | sort | tail -n1 2>/dev/null)
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion adds a sort pipe to stabilize the output when multiple tags exist. However, it may change the intended ordering and is a minor improvement.

    Low

    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.

    1 participant