feat(apps/prod/tekton/configs): debug build for mac targets

[wuhuizuo]

Current it in debug stage.

[ti-chi-bot]

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

Pull Request Review

Title: feat(apps/prod/tekton/configs): debug build for mac targets
Description: Current it in debug stage.

Key Changes

This pull request adds a new optional secret mac-ssh-credentials to the push-build-package.yaml and pingcap-build-binaries.yaml files. It also modifies the push-build-package.yaml file to modify the os parameter to darwin for mac targets and adds a new task build-images to build linux targets. Finally, it modifies the pingcap-build-binaries.yaml file to add a new env parameter and a new if statement to differentiate between mac and linux targets.

Potential Problems

• The pull request description is not very informative. It is unclear what the debug stage refers to.
• It is unclear how to use the new mac-ssh-credentials secret. Some documentation or instructions should be added to the pull request description or the code comments.
• There is a typo in the pingcap-build-binaries.yaml file, where secert should be secret in the mac-ssh-credentials secret.
• The build-images task in push-build-package.yaml is only run for linux targets. This may not be desirable if mac targets also need to build images.

Fixing Suggestions

• The pull request description should be updated to provide more detail on the changes made and how to use the new mac-ssh-credentials secret.
• The typo in the pingcap-build-binaries.yaml file should be corrected.
• Consider modifying the build-images task in push-build-package.yaml to also build images for mac targets if necessary.

[ti-chi-bot]

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

This Pull Request adds support for building debug versions of the application for Mac targets. The changes include adding a new secret named mac-ssh-credentials, modifying the push-build-package.yaml and pingcap-build-binaries.yaml files to use it, and adding code to handle building on Mac targets.

Potential problems:

1. There are a few typos in the description and the code comments that should be fixed before merging.
2. The new code for building on Mac targets is not tested, which could lead to issues down the line.
3. The pingcap/tidb/push.yaml, pingcap/tiflash/push.yaml, pingcap/tiflow/push.yaml, tikv/pd/tag-build.yaml, and tikv/tikv/push.yaml files are commented out in kustomization.yaml, but it's not clear why. This should be explained in the pull request or the comments should be removed.

Fixing suggestions:

1. Fix the typos in the description and code comments.
2. Add tests for the new Mac target code.
3. Either explain why the commented-out files are not used or remove the comments.

[ti-chi-bot]

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

Review of Pull Request "feat(apps/prod/tekton/configs): debug build for mac targets"

Summary

This pull request adds the ability to build binaries for macOS targets in a debugging mode. The changes include:

• Adding a new mac-ssh-credentials secret which contains the SSH private key for logging in to a macOS machine.
• Modifying the push-build-package.yaml and pingcap-build-binaries.yaml files to add support for building macOS targets.
• Adding an SSH command to the pingcap-build-binaries.yaml file that uses the SSH private key to log in to a macOS machine and build the binaries.

Potential Problems

• The when clause in pingcap-build-binaries.yaml only checks if the operating system is linux before building. This could cause issues if the os parameter is not set correctly to darwin for macOS builds.
• The SSH command in pingcap-build-binaries.yaml assumes that the username, host, and workspace_base_dir are predefined. It should either use environment variables or provide a way to set these values dynamically.
• The changes to kustomization.yaml are commented out, which is not ideal. It is unclear why these changes were commented out and whether they should be enabled or not.

Suggestions

• Update the when clause in pingcap-build-binaries.yaml to ensure that only macOS targets are built when the os parameter is set to darwin.
• Use environment variables or provide a way to set username, host, and workspace_base_dir dynamically in the SSH command in pingcap-build-binaries.yaml.
• Clarify why the changes to kustomization.yaml were commented out and whether they should be enabled or not.

[wuhuizuo]

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

Pull Request Review

Title: feat(apps/prod/tekton/configs): debug build for mac targets Description: Current it in debug stage.

Key Changes

This pull request adds a new optional secret mac-ssh-credentials to the push-build-package.yaml and pingcap-build-binaries.yaml files. It also modifies the push-build-package.yaml file to modify the os parameter to darwin for mac targets and adds a new task build-images to build linux targets. Finally, it modifies the pingcap-build-binaries.yaml file to add a new env parameter and a new if statement to differentiate between mac and linux targets.

Potential Problems

• The pull request description is not very informative. It is unclear what the debug stage refers to.
• It is unclear how to use the new mac-ssh-credentials secret. Some documentation or instructions should be added to the pull request description or the code comments.
• There is a typo in the pingcap-build-binaries.yaml file, where secert should be secret in the mac-ssh-credentials secret.
• The build-images task in push-build-package.yaml is only run for linux targets. This may not be desirable if mac targets also need to build images.

Fixing Suggestions

• The pull request description should be updated to provide more detail on the changes made and how to use the new mac-ssh-credentials secret.
• The typo in the pingcap-build-binaries.yaml file should be corrected.
• Consider modifying the build-images task in push-build-package.yaml to also build images for mac targets if necessary.

Thanks, you help me found the typo issue.

[wuhuizuo]

/approve

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

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:

• apps/prod/OWNERS [wuhuizuo]

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