Work CI-CD #3154
Work CI-CD #3154
Conversation
WalkthroughThe Azure Pipelines YAML configuration has been modified by commenting out several build jobs for various targets (STM32, ESP32, NXP, Azure RTOS, WIN32, and nanoCLR CLI). Additionally, the Changes
Sequence Diagram(s)Old Pipeline FlowsequenceDiagram
participant P as Pipeline Trigger
participant B as Build Jobs (STM32, ESP32, NXP, Azure RTOS, WIN32, nanoCLR CLI)
participant R as Report_Build_Failure
P->>B: Trigger build jobs
B-->>P: Return build results
P->>R: Forward build results for failure report
New Pipeline FlowsequenceDiagram
participant P as Pipeline Trigger
participant R as Report_Build_Failure
P->>R: Trigger report without build job dependencies
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
azure-pipelines.yml (2)
366-713: Non‑TI Build Jobs Removed from the Pipeline
The diff shows that multiple build jobs for targets not related to TI (such as STM32, ESP32, NXP, Azure RTOS, WIN32, and nanoCLR CLI) have been commented out. This effectively eliminates these builds from the CI/CD process and narrows the focus to TI SimpleLink targets, which aligns with the PR objective.A couple of suggestions:
- If these targets will no longer be built, it may be cleaner in the long run to remove the commented‐out code entirely instead of leaving it in place.
- If you intend to re-enable these jobs later, please add a comment clarifying that these are temporarily disabled to help future maintainers.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 654-654: trailing spaces
(trailing-spaces)
1447-1472: Refine Dependency and Condition in the Report_Build_Failure Job
In the Report_Build_Failure job, the dependency now only includesBuild_TI_SimpleLink_targets(as intended after removing non‑TI build jobs), which is appropriate given the current pipeline focus. However, note that the condition expression shows a trailing comma inside theor()clause:- failed('Build_TI_SimpleLink_targets'), + failed('Build_TI_SimpleLink_targets')Removing the trailing comma can help avoid potential YAML parsing or evaluation issues. Please verify that the updated condition correctly reports failures based solely on the TI build’s status.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
azure-pipelines.yml(2 hunks)
🔇 Additional comments (1)
azure-pipelines.yml (1)
1-35: Overall Pipeline Configuration Remains Consistent with CI/CD Objectives
The rest of the pipeline configuration (triggers, resource references, and the initial build options) remains unaffected. This helps ensure that the changes—focusing exclusively on the TI build—integrate well into the overall CI/CD strategy without introducing side effects.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
- Test TI build.
josesimoes
force-pushed
the
test-ti-build
branch
from
a89f136 to
1a5a791
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
azure-pipelines.yml (1)
366-713: Temporary Disabling of Non-TI Build Jobs
A large section of the pipeline (from STM32, ESP32, NXP, Azure RTOS, WIN32, and nanoCLR CLI targets) has been commented out. This aligns with the PR objective of isolating and testing the TI build. Please ensure that:
- This change is indeed temporary (or documented as permanent if intended) so that future maintainers understand the rationale.
- A comment is added to explain that these jobs are intentionally disabled to focus on TI builds for CI/CD testing.
Suggested diff for an inline note:
@@ - # ############### + # ############### + # TEMPORARILY DISABLED: Non-TI build jobs are commented out + # for CI/CD testing purposes as per PR #3154. Re-enable when appropriate.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 654-654: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
azure-pipelines.yml(2 hunks)
🔇 Additional comments (2)
azure-pipelines.yml (2)
636-713: TI SimpleLink Build Job Verification
The active jobBuild_TI_SimpleLink_targetsappears configured with a matrix (e.g. target TI_CC1352R1_LAUNCHXL_915 with specific build options). Verify that the TI build options (such as-DRADIO_FREQUENCY=915and the designated target series) are up‑to‑date and reflect the intended configuration for testing the TI build.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 654-654: trailing spaces
(trailing-spaces)
1447-1463: Report Build Failure Job Dependency Update
TheReport_Build_Failurejob now depends solely onCheck_Code_StyleandBuild_TI_SimpleLink_targets(instead of the previously included jobs now commented out). This simplified dependency is consistent with the removal of other build jobs. Please verify that this dependency change still captures all failure scenarios relevant for the CI/CD pipeline.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
azure-pipelines.yml (2)
366-635: Disabled Non-TI Build Jobs (STM32, ESP32, NXP, etc.)
Several build jobs—including those for STM32, ESP32, and NXP targets—are now completely commented out in this section. This appears to be an intentional change to restrict the pipeline to only the TI build for this test.Recommendation: Please ensure that this temporary disablement is well documented (in the commit message or supporting docs) so that these jobs can be re-enabled easily when needed. Alternatively, consider using a pipeline variable or conditional inclusion instead of commenting out large blocks of YAML to improve maintainability.
712-1470: Disabled Additional Reporting and Publishing Jobs
From this point onward, jobs related to Azure RTOS, WIN32 executable, nanoCLR CLI, mscorlib unit tests, changelog generation, and the build failure reporting step have been commented out. This streamlines the pipeline to focus solely on the TI build.Recommendation: Verify that leaving these jobs disabled does not adversely affect any downstream processes (for example, artifact publishing or notifications) that might be expected in a full build. If these jobs will be re-enabled later, consider using feature flags or conditional YAML inclusion rather than commenting them out to ease maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
azure-pipelines.yml(2 hunks)
🔇 Additional comments (1)
azure-pipelines.yml (1)
636-712: Enabled TI SimpleLink Build Job
The TI SimpleLink target is now the only active build job in this section. This aligns with the PR objective to test the TI build.Recommendation: Confirm that all environment variables, toolchain settings, and any TI-specific configurations are still current and sufficient for your testing needs.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 654-654: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
azure-pipelines-templates/download-install-ninja.yml (1)
35-37: Enhance the Ninja Version Check stepThe newly added step that runs
ninja --versionis a straightforward and effective way to ensure that Ninja is correctly installed and accessible via the PATH. This check helps catch misconfigurations early in the pipeline.Optionally, consider verifying that the output meets a specific minimum version requirement if one is critical for your builds, or add explicit error handling to provide a more descriptive failure message when the command fails.
Description
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist
Summary by CodeRabbit