-
Notifications
You must be signed in to change notification settings - Fork 5k
fix(build): disable tests during build process for tdengine build ci #33338
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @tomchon, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a mechanism to control whether tests are built as part of the main project compilation. By making the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR disables test building during the TDengine build CI process to optimize build times. The changes introduce a BUILD_TEST environment variable that defaults to true for backward compatibility but can be set to false in CI workflows.
Key changes:
- Added
BUILD_TESTenvironment variable support in build scripts with default value oftrue - Updated CI workflow to explicitly set
BUILD_TEST=falseduring all build steps
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| build.sh | Modified to use BUILD_TEST environment variable instead of hardcoded true value |
| build.bat | Added BUILD_TEST environment variable with default value and updated cmake configuration |
| .github/workflows/tdengine-build.yml | Set BUILD_TEST=false for all build steps in Linux, macOS, and Windows builds |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a mechanism to control test builds via the BUILD_TEST environment variable in both build.bat and build.sh. The changes correctly default the variable to true if it's not set, allowing it to be overridden (e.g., in a CI environment) to disable building tests. The implementation is idiomatic for both batch and shell scripts. I've made one minor suggestion to fix the alignment in build.sh for consistency. Otherwise, the changes look good.
…matting in build.sh
No description provided.