Skip to content

Conversation

@mernst
Copy link
Member

@mernst mernst commented Oct 13, 2025

Summary by CodeRabbit

  • Chores

    • Standardized and parameterized CI cache key prefixes and updated save/restore references across CircleCI and Azure pipelines.
    • Consolidated/compactified checkout logic and applied whitespace/indentation/formatting cleanup across pipeline definitions.
    • Added file-permission toggles around generation steps.
    • Disabled one typecheck job by converting it to a commented configuration and fixed a multi-line comment formatting.
  • Tests

    • Added additional typecheck jobs for Ubuntu 21 and 25.

@coderabbitai
Copy link

coderabbitai bot commented Oct 13, 2025

📝 Walkthrough

Walkthrough

Renames and parameterizes CircleCI cache anchors/keys and replaces anchor references; compacts checkout logic in CircleCI macros. Adjusts Azure pipeline/job YAML formatting and comments (disables one typecheck job). Adds Makefile permission toggles around m4. No public API or functional code changes.

Changes

Cohort / File(s) Summary of Changes
CircleCI — cache anchor/key renames
.circleci/config.yml
Replaced cache key prefixes and anchor names (e.g., source-v1-…/&source-cachesourcefull-v1full-…/&sourcefull-cache) across restore_cache/save_cache usages; comment formatting tweak in typecheck workflow.
CircleCI — parameterized macros & checkout
.circleci/defs.m4
Parameterized cache anchor/key to include macro arg (source$1-cache / source$1-v1$1-…), updated save_cache references, and replaced conditional checkout blocks with an inline checkout[]ifelse($1,full,[:: method: full]) expression.
Azure pipelines — YAML shape & job formatting
.azure/azure-pipelines.yml, .azure/azure-pipelines.yml.m4, .azure/defs.m4, .azure/jobs.m4
Reflowed/indented job blocks and list structure, changed testingSuffix representation, reformatted/commented out the typecheck_onefile_jdk21 job (disabled via commenting), and added Ubuntu typecheck job variants; minor comment indentation edits.
Makefile — permission toggling around m4
.azure/Makefile
Added quiet chmod to make targets writable before invoking m4 (`-f +w $@

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as Developer
    participant CI as CircleCI
    participant Job as Job
    participant Cache as Cache

    Dev->>CI: push / open PR
    CI->>Job: start job
    rect rgb(240,248,255)
      note right of Job: restore_cache uses renamed/parameterized keys/anchors
      Job->>Cache: restore_cache: e.g. sourcefull-v1full-{{ .Branch }}-{{ .Revision }}\nfallbacks: sourcefull-v1full-{{ .Branch }}-, sourcefull-v1full-
      Cache-->>Job: hit / miss
    end
    Job->>Job: checkout (inline ifelse emits full checkout when macro arg == "full")
    Job->>Job: execute steps (build/test)
    Job->>Cache: save_cache: matching parameterized key/anchor
    Cache-->>Job: saved
    Job-->>CI: complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the primary change—introducing a distinct cache key for full Git clones—and directly reflects the modifications to cache key prefixes and anchor names across CI configurations. It is concise and specific, avoiding unnecessary details or vagueness, enabling team members to understand the PR’s intent at a glance.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ff09cf and 8dc10de.

📒 Files selected for processing (3)
  • .circleci/config.yml (11 hunks)
  • .circleci/config.yml.m4 (1 hunks)
  • .circleci/defs.m4 (1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.circleci/config.yml

[warning] 1320-1320: comment not indented like content

(comments-indentation)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: codespecs.daikon (kvasir_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (typecheck_latest_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (typecheck_bundled_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (nonquick_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (misc_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (nontxt_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (quick_ubuntu_jdk25)
  • GitHub Check: build
🔇 Additional comments (4)
.circleci/config.yml (2)

326-332: Cache key separation for full clones implemented correctly.

The cache keys for jobs using full checkout now use the sourcefull-v1full- prefix with the &sourcefull-cache anchor, which correctly separates them from shallow clone caches (source-v1-). This pattern is consistently applied across all misc-* jobs.


1320-1321: Comment brace mismatch corrected.

The comment now uses matching braces {8,11} instead of the previous {8,11).

.circleci/config.yml.m4 (1)

61-61: Comment brace mismatch corrected.

The template now generates the comment with matching braces {8,11} instead of {8,11).

.circleci/defs.m4 (1)

18-24: Cache key parameterization correctly implemented.

The circleci_boilerplate macro now accepts a parameter to generate distinct cache keys for full vs shallow clones:

  • When called without arguments: generates source-v1- keys for shallow clones
  • When called with full: generates sourcefull-v1full- keys and adds method: full to checkout

This prevents cache collisions between different checkout types.


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

@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

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6183e60 and 76ce790.

📒 Files selected for processing (1)
  • .circleci/defs.m4 (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: codespecs.daikon (quick_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (typecheck_latest_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (typecheck_bundled_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (nontxt_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (nonquick_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (misc_ubuntu_jdk25)
  • GitHub Check: codespecs.daikon (kvasir_ubuntu_jdk25)
  • GitHub Check: build
🔇 Additional comments (1)
.circleci/defs.m4 (1)

18-20: Cache key parameterization looks correct.

The addition of $1 to the cache key prefix effectively creates separate cache namespaces for full and shallow clones, which aligns with the PR objective. When circleci_boilerplate(full) is invoked (as in misc_job), keys become source-v1full-...; otherwise, they remain source-v1-....

Copy link

@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: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 76ce790 and be5da43.

📒 Files selected for processing (6)
  • .azure/Makefile (1 hunks)
  • .azure/azure-pipelines.yml (1 hunks)
  • .azure/azure-pipelines.yml.m4 (1 hunks)
  • .azure/defs.m4 (1 hunks)
  • .azure/jobs.m4 (1 hunks)
  • .circleci/config.yml (10 hunks)
🔇 Additional comments (15)
.circleci/config.yml (9)

348-350: Consistent v1full keys here as well.

Matches the full checkout usage.


370-372: v1full key prefix looks correct.


392-394: Full-clone cache keys updated correctly.


414-416: Consistent with intended full-clone cache separation.


990-992: Rocky misc job uses v1full keys as expected.


1012-1014: v1full cache keys applied correctly.


1034-1036: Correct v1full usage for full checkout.


1056-1058: Cache prefix update aligns with full clone.


1078-1080: Consistent v1full keys; looks good.

.azure/jobs.m4 (1)

31-38: LGTM! Expanded typecheck coverage.

The addition of typecheck jobs for Ubuntu 21 and 25 follows the existing pattern and expands test coverage appropriately. The comment formatting improvement enhances readability.

.azure/defs.m4 (1)

11-146: LGTM! Formatting improvements.

The consistent indentation applied across all job definition macros significantly improves readability without changing any functional behavior.

.azure/azure-pipelines.yml.m4 (3)

18-19: LGTM! Improved variable declaration format.

The list-based variable declaration format is cleaner and aligns with YAML best practices.


23-41: LGTM! Enhanced canary jobs structure.

The reformatted canary_jobs block with improved indentation and explicit dependency list significantly improves readability.


45-59: Verify that disabling the typecheck_onefile_jdk21 job is intentional.

The typecheck_onefile_jdk21 job has been commented out. According to the comment, this job was used to "prioritize typechecking a single file, without waiting for other tests." Please confirm that this debugging/development feature is no longer needed.

.azure/azure-pipelines.yml (1)

775-789: Note: This is a generated file.

This file is generated from .azure/azure-pipelines.yml.m4 by the m4 preprocessor. The commented-out typecheck_onefile_jdk21 job reflects the template changes already reviewed. Any modifications should be made to the .m4 template file, not directly to this file.

Copy link

@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: 2

♻️ Duplicate comments (1)
.circleci/config.yml (1)

326-328: Inconsistent cache key prefix (duplicate of .circleci/defs.m4 issue).

Line 326 uses source-v1full- while lines 327-328 use sourcefull-v1full-. This mismatch is generated by the pattern inconsistency in .circleci/defs.m4 line 18 and will prevent proper cache restoration.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be5da43 and 8ff09cf.

📒 Files selected for processing (2)
  • .circleci/config.yml (11 hunks)
  • .circleci/defs.m4 (1 hunks)
🔇 Additional comments (3)
.circleci/defs.m4 (2)

21-22: Inline conditional for checkout is correctly structured.

The ifelse macro correctly produces either a plain checkout step or a checkout step with method: full based on the parameter. The generated YAML structure in .circleci/config.yml confirms valid formatting.


24-24: Parameterized cache key reference correctly separates full and shallow clone caches.

The change from *source-cache to *source$1-cache properly references the parameterized anchor, preventing cache collisions between full and shallow clones.

.circleci/config.yml (1)

28-28: Duplicate anchor declarations remain but are reduced.

The &source-cache anchor is still declared in multiple jobs throughout the file (lines 28, 48, 68, 88, 108, 129, 149, 169, 189, 209, 230, 249, 268, 287, 306, etc.). While YAML parsers may tolerate this by allowing later definitions to overwrite earlier ones, it technically violates the YAML specification.

However, this PR improves the situation by introducing &sourcefull-cache for full-clone jobs, reducing the number of duplicate &source-cache declarations compared to the previous version.

For a complete fix, consider parameterizing anchor names per job in the M4 templates (e.g., &source-cache-<job-name>) to ensure each anchor is unique. This would require updating the M4 macro definition in .circleci/defs.m4.

@mernst mernst merged commit 88de63e into codespecs:master Oct 14, 2025
117 checks passed
@mernst mernst deleted the circleci-git-cache branch October 14, 2025 01:28
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