search

PingCAP-QE / ci

Continue intergration tests
Apache License 2.0
19 stars 96 forks source link

fix: handle non-standard version information of dumpling #2991

Closed purelind closed 2 weeks ago

purelind commented 2 weeks ago

handle non-standard version information of dumpling on release-check-version.

ti-chi-bot[bot] commented 2 weeks ago

I have already done a preliminary review for you, and I hope to help you do a better job.

This pull request is aimed at fixing a non-standard version information issue for the dumpling component in the release-check-version script. The changes involve adding comments to explain the issue and adding a fix to handle the situation where the version information is output to stderr instead of stdout.

The potential problem with this pull request is that the fix is specific to the dumpling component and may not handle similar issues with other components. Also, the change made to the check_offline_package.py file may cause an error as the join() method requires a sequence of strings, but only two strings are being passed as arguments.

To fix this, the reviewer could suggest making the logic more generic to handle similar issues with other components. Additionally, the reviewer could suggest using the + operator instead of the join() method in the check_offline_package.py file to concatenate the two strings.

ti-chi-bot[bot] commented 2 weeks ago

I have already done a preliminary review for you, and I hope to help you do a better job.

From the pull request, it seems that the change is related to handling non-standard version information of dumpling on release-check-version. The change includes adding a comment to reference the issue and modifying the code in three files to combine stdout and stderr for dmctl and dumpling.

As for potential problems, the change seems to be a minor one and does not pose any immediate issues. However, it is always a good practice to thoroughly test the change before merging it.

As for fixing suggestions, it is recommended to add test cases to validate the change and ensure that it works as expected. Moreover, the commit message could be more descriptive and informative, providing more context about the issue being fixed.

ti-chi-bot[bot] commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

The pull request process is described here

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/PingCAP-QE/ci/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
wuhuizuo commented 2 weeks ago

/lgtm