Skip to content

Add canary job#263

Merged
mernst merged 3 commits intotypetools:masterfrom
mernst:ci-depends-on
Apr 12, 2026
Merged

Add canary job#263
mernst merged 3 commits intotypetools:masterfrom
mernst:ci-depends-on

Conversation

@mernst
Copy link
Copy Markdown
Member

@mernst mernst commented Apr 12, 2026

No description provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 12, 2026

📝 Walkthrough

Walkthrough

Three Azure Pipeline files were changed to add a synthetic orchestration job and make downstream jobs wait for it. A new canary_jobs job was added to .azure/azure-pipelines.yml and .azure/azure-pipelines.yml.m4 (depends on build_jdk and build_jdk21u). Multiple downstream jobs in .azure/azure-pipelines.yml, .azure/azure-pipelines.yml.m4, and .azure/defs.m4 were updated to include dependsOn: [canary_jobs] at the job level; YAML indentation/structure for several jobs was adjusted. A new M4 macro jdku_version = 21u was added to .azure/defs.m4.

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.azure/defs.m4 (2)

79-88: ⚠️ Potential issue | 🟠 Major

Missing dependsOn for inference_part2_jdk$1.

Unlike inference_part1_jdk$1 which depends on canary_jobs, inference_part2_jdk$1 has no dependsOn clause. This means it will start immediately without waiting for the build jobs to complete, which appears inconsistent with the orchestration pattern established in this PR.

🔧 Proposed fix
   - job: inference_part2_jdk$1
+    dependsOn:
+      - canary_jobs
     pool:
       vmImage: 'ubuntu-latest'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.azure/defs.m4 around lines 79 - 88, Add a dependsOn clause to the job
inference_part2_jdk$1 so it mirrors the orchestration of inference_part1_jdk$1;
specifically, update the job definition for inference_part2_jdk$1 to include
dependsOn: canary_jobs (or the same dependency list used by
inference_part1_jdk$1) so the job waits for the build/canary jobs to finish
before running.

136-143: ⚠️ Potential issue | 🟠 Major

Missing dependsOn for typecheck_part2_jdk$1.

Same issue as inference_part2_jdk$1 — this job has no dependsOn clause while typecheck_part1_jdk$1 (lines 125-127) explicitly depends on canary_jobs. This creates an inconsistency where part2 can start before builds complete.

🔧 Proposed fix
   - job: typecheck_part2_jdk$1
+    dependsOn:
+      - canary_jobs
     pool:
       vmImage: 'ubuntu-latest'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.azure/defs.m4 around lines 136 - 143, Add a dependsOn to the job definition
for typecheck_part2_jdk$1 so it cannot start before the preceding
typecheck_part1_jdk$1 (which currently depends on canary_jobs); specifically,
update the job block for typecheck_part2_jdk$1 to include a dependsOn:
typecheck_part1_jdk$1 entry to enforce correct ordering (similar to how
inference_part2_jdk$1 was fixed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.azure/defs.m4:
- Line 10: The macro `jdku_version` is defined but unused; either delete the
define([jdku_version], [21u])dnl line to remove dead code, or if you intend to
keep it for future use, replace it with the same definition plus a clear TODO
comment on its purpose and planned usage (e.g., TODO: reserved for future JDK/U
versioning) so reviewers know it’s intentional; locate the `jdku_version`
definition in the M4 definitions file and apply one of these two changes.

---

Outside diff comments:
In @.azure/defs.m4:
- Around line 79-88: Add a dependsOn clause to the job inference_part2_jdk$1 so
it mirrors the orchestration of inference_part1_jdk$1; specifically, update the
job definition for inference_part2_jdk$1 to include dependsOn: canary_jobs (or
the same dependency list used by inference_part1_jdk$1) so the job waits for the
build/canary jobs to finish before running.
- Around line 136-143: Add a dependsOn to the job definition for
typecheck_part2_jdk$1 so it cannot start before the preceding
typecheck_part1_jdk$1 (which currently depends on canary_jobs); specifically,
update the job block for typecheck_part2_jdk$1 to include a dependsOn:
typecheck_part1_jdk$1 entry to enforce correct ordering (similar to how
inference_part2_jdk$1 was fixed).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3838c6bc-9668-4028-be68-79fea80db082

📥 Commits

Reviewing files that changed from the base of the PR and between 7d26e2d and 8fd7a26.

📒 Files selected for processing (3)
  • .azure/azure-pipelines.yml
  • .azure/azure-pipelines.yml.m4
  • .azure/defs.m4

Comment thread .azure/defs.m4 Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.azure/defs.m4:
- Around line 180-195: The macro plume_lib_job declares a parameter ($1) but
never uses it; either remove the parameter from invocations of
plume_lib_job(canary_version) or use $1 inside the macro (for example substitute
it into the container image tag like container: mdernst/cf-ubuntu-jdk17:$1) so
the passed version is applied, or add an explicit comment inside the
plume_lib_job macro body noting that $1 is intentionally unused for consistency;
update the macro definition (plume_lib_job) and all its call sites to match the
chosen approach.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 24aeea9d-ce99-4727-adb2-c071f1d402f3

📥 Commits

Reviewing files that changed from the base of the PR and between 8fd7a26 and 6ee41a6.

📒 Files selected for processing (1)
  • .azure/defs.m4

Comment thread .azure/defs.m4
@mernst mernst merged commit 4419154 into typetools:master Apr 12, 2026
20 checks passed
@mernst mernst deleted the ci-depends-on branch April 12, 2026 21:23
@coderabbitai coderabbitai Bot mentioned this pull request Apr 25, 2026
@coderabbitai coderabbitai Bot mentioned this pull request May 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant