Skip to content
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

support go1.23 for tidb master branch #3154

Merged
merged 3 commits into from
Oct 9, 2024
Merged

Conversation

hawkingrei
Copy link
Contributor

No description provided.

Copy link

ti-chi-bot bot commented Oct 8, 2024

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

The Pull Request titled "support go1.23" seems to be a straightforward update to change the version of Go being used in various pipelines and tests, specifically within the context of the Golang containers that are being used. The changes only involve updating the image from "hub.pingcap.net/wangweizhen/tidb_image:go12120230809" to "hub.pingcap.net/wangweizhen/tidb_image:go12320241008".

There is no apparent problem at first glance, but here are some potential issues and suggestions:

  1. Dependency compatibility: The new Go version might have changes that can affect the project's dependencies or the project itself. You should ensure that all dependencies are compatible with Go 1.23, and run all tests to ensure everything still works as expected.

  2. Testing: Ensure that all test cases are passing after the Go version upgrade. If any test fails, identify the cause and fix the issue.

  3. Update Documentation: If there are any documents mentioning the Go version, they should be updated to avoid confusion.

  4. Container Image Verification: Ensure that the new image "hub.pingcap.net/wangweizhen/tidb_image:go12320241008" is available and correctly configured with Go 1.23. If not, it might cause runtime errors.

  5. Missing PR Description: Although the title is self-explanatory, it is a good practice to provide a brief description in the PR about the changes and the reason for the upgrade.

@ti-chi-bot ti-chi-bot bot added the size/M label Oct 8, 2024
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
Copy link

ti-chi-bot bot commented Oct 8, 2024

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

This pull request changes the version of the Go image used in the Jenkins pipeline for the TiDB project from go12120230809 to go12320241008. The change affects multiple YAML files in different directories.

There are no potential problems with this change.

However, it is good practice to ensure that the new version of the Go image is compatible with the TiDB project. It would be best to run tests to ensure that the code compiles and runs correctly with this new image.

Also, it is recommended to update the pull request description to include more information about why this change is necessary.

Overall, the changes look good.

@ti-chi-bot ti-chi-bot bot added size/S and removed size/M labels Oct 8, 2024
@ti-chi-bot ti-chi-bot bot added size/M and removed size/S labels Oct 8, 2024
Copy link

ti-chi-bot bot commented Oct 8, 2024

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

The pull request titled "support go1.23" is aiming to update the Go version from 1.21 to 1.23 in various Jenkins pipelines and test environments.

Key Changes:

  • The Go version is updated from 1.21 to 1.23 in various files such as Jenkins pipeline scripts, and Kubernetes pod definitions related to CI/CD pipelines and test environments.
  • The image used for the 'golang' container in these files is updated to a new version which presumably has Go 1.23 installed.

Potential Problems:

  1. The updated image tags are pointing to a specific user's repository ("hub.pingcap.net/wangweizhen/tidb_image:go12320241008"). This could be problematic if the user deletes the repository or the specific tag. A more stable, organization-maintained Docker image might be a better choice.
  2. There's no information whether the Go 1.23 version is fully compatible with the existing codebase. If there are breaking changes introduced in Go 1.23 which are not handled in the code, it might lead to build or runtime errors.
  3. The pull request lacks a detailed description explaining the need for the change, whether any testing has been done, etc.

Fixing Suggestions:

  1. Use a more stable, organization-maintained Docker image for Go 1.23.
  2. Thoroughly test the changes in a separate environment before merging the pull request, ensuring Go 1.23 is compatible with the existing codebase.
  3. Update the pull request description with more details, including the reason for the change, testing done, any potential impact, etc.

@purelind
Copy link
Collaborator

purelind commented Oct 8, 2024

/hold

wait for PR pingcap/tidb#51126

Copy link

ti-chi-bot bot commented Oct 8, 2024

[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:

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

@purelind purelind changed the title support go1.23 support go1.23 for tidb master branch Oct 8, 2024
@purelind
Copy link
Collaborator

purelind commented Oct 9, 2024

/unhold

@ti-chi-bot ti-chi-bot bot merged commit 7c6287f into PingCAP-QE:main Oct 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants