Skip to content

Add CI log grouping for align-deps output#4150

Open
vivekjm wants to merge 2 commits into
microsoft:mainfrom
vivekjm:add-align-deps-ci-groups
Open

Add CI log grouping for align-deps output#4150
vivekjm wants to merge 2 commits into
microsoft:mainfrom
vivekjm:add-align-deps-ci-groups

Conversation

@vivekjm
Copy link
Copy Markdown

@vivekjm vivekjm commented May 13, 2026

Summary

Adds CI log grouping around align-deps output so runs over larger workspaces are easier to scan in GitHub Actions and Azure DevOps.

This keeps local output unchanged and only opens a group when a package actually writes log output, avoiding empty groups for packages with no changes.

Fixes #2906.

Testing

  • git diff --check
  • yarn workspace @rnx-kit/align-deps format
  • PATH="/Users/vivek/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH" yarn workspace @rnx-kit/align-deps lint
  • PATH="/Users/vivek/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH" yarn workspace @rnx-kit/align-deps build --dependencies
  • PATH="/Users/vivek/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH" yarn workspace @rnx-kit/align-deps test

@vivekjm vivekjm changed the title Group align-deps logs on CI Add CI log grouping for align-deps output May 13, 2026
Copy link
Copy Markdown
Member

@tido64 tido64 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I've left some comments.

Comment thread packages/align-deps/src/ci.ts Outdated

export function logGroupFor(
title: string,
env: Env = process.env
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Type can be inferred

Suggested change
env: Env = process.env
env = process.env

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good call, thanks. I removed the explicit env type and let TypeScript infer it from the default value.

Comment thread packages/align-deps/src/ci.ts Outdated
): LogGroup | undefined {
if (env["GITHUB_ACTIONS"] === "true") {
return {
start: `::group::${escapeGitHubCommand(title)}`,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should use encodeURI instead:

Suggested change
start: `::group::${escapeGitHubCommand(title)}`,
start: `::group::${encodeURI(title)}`,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. I switched the GitHub Actions group title handling to use encodeURI here.

Comment thread packages/align-deps/src/cli.ts Outdated
// disk only when everything is in order for the target package. Packages with
// invalid or missing configurations are skipped.
const errors = manifests.reduce((errors, manifest) => {
const logGroup = logGroupFor(manifest);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we always return start/end functions instead?

  const { beginGroup, endGroup } = getGroupMarkers();
  const errors = manifests.reduce((errors, manifest) => {
    try {
      beginGroup(manifest);
      ...
    } finally {
      endGroup(manifest);
    }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated this to expose beginGroup/endGroup through getGroupMarkers. I also moved the call site behind a small helper so the CLI loop stays readable.

Comment thread packages/align-deps/src/cli.ts Outdated
Comment on lines +272 to +274
if (logGroup) {
console.log(logGroup.start);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is correct. This will always start a group even though we don't know if anything will be logged yet. When no changes are needed, nothing should be logged.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed. I changed the implementation so the group starts lazily on the first log/warn/error for that manifest. If nothing is logged, no group markers are emitted.

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.

align-deps: add support for grouping log lines on CI

2 participants