Skip to content

refactor: unify CI detection - #2103

Open
rix0rrr wants to merge 2 commits into
mainfrom
huijbers/ci-providers
Open

rix0rrr wants to merge 2 commits into
mainfrom
huijbers/ci-providers

Conversation

@rix0rrr

@rix0rrr rix0rrr commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Move all the CI detection logic into one place (used to be in 3), add a couple more CI providers to the detection list.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Move all the CI detection logic into one place (used to be in 3),
add a couple more CI providers to the detection list.
@rix0rrr
rix0rrr requested a review from a team October 28, 2026 16:32
@rix0rrr
rix0rrr deployed to automation October 28, 2026 16:33 — with GitHub Actions Active
@github-actions github-actions Bot added the p2 label Oct 28, 2026
@rix0rrr
rix0rrr deployed to no-approval October 28, 2026 16:33 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@maintainer-for-aws

maintainer-for-aws Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Automated review

A maintainer will still review this — treat the notes below as a starting point.

📌 Commit 0501b8b

This PR consolidates CI detection in the CDK CLI toolchain: it relocates ci-systems.ts into @aws-cdk/toolkit-lib, adds a single unified isCI() there, removes the two duplicate isCI() definitions (in lib/cli/util/ci.ts and shell-env.ts), re-exports the symbols to the CLI through api-private, and repoints every consumer. It also adds four CI providers (Bamboo, BuildKite, Travis, Heroku) to the detection table. The only substantive logic change is that the unified isCI() now falls back to detectCiSystem() when CI is unset, generalizing a pattern already present in the telemetry path; the relocated symbols stay internal and are not added to the toolkit-lib public API. The refactor is clean and consistent with the repo's layering. One minor doc-accuracy nit on the isCI prop comment is noted inline.

🔴 0 blocking · 🟡 0 recommended · ⚪ 1 optional

Files with findings (1)
File Findings
packages/@aws-cdk/toolkit-lib/lib/toolkit/non-interactive-io-host.ts ⚪ 1

Generated automatically. React 👍 or 👎 to tell us whether this review helped, so we can improve these reviews.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.00%. Comparing base (d08ccc1) to head (0501b8b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2103      +/-   ##
==========================================
- Coverage   92.08%   92.00%   -0.08%     
==========================================
  Files          83       81       -2     
  Lines       13246    13153      -93     
  Branches     1886     1875      -11     
==========================================
- Hits        12197    12101      -96     
- Misses       1011     1014       +3     
  Partials       38       38              
Flag Coverage Δ
suite.unit 92.00% <100.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@9pace 9pace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This also changes the behavior of the ci detection such that now we will automatically do --ci behavior if a ci env var is detected, whereas before the detection only impacted telemetry.

could you clarify if this is the intent of the PR?

This branch had an error being deployed

1 failed (outdated), 1 active, and 1 inactive deployments
run-tests — 0501b8b7 Deployed Oct 29, 2026 by rix0rrr via integ_init-templates (init-typescript-app, 20) #7515
no-approval — 0501b8b7 Deployed Oct 29, 2026 by rix0rrr via prepare #7515
automation — 68e628d9 Deployed Oct 28, 2026 by rix0rrr via Triage Pull Requests #2260
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants