fix(github-actions): resolve pending mergeability checks on LTS branches#3714
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the branch manager local action by wrapping the execution logic inside an async main function, improving configuration loading and validation, and ensuring proper token revocation in a finally block. It also updates setCachedConfig to accept null. The review comments identify a critical issue where using the nullish coalescing operator (??=) on sha will fail to fall back when the input is an empty string, and suggest removing a redundant check for token !== undefined in the finally block.
alan-agius4
force-pushed
the
lts-fix
branch
4 times, most recently
from
1aabecb to
3c1c489
Compare
josephperrott
left a comment
josephperrott
left a comment
There was a problem hiding this comment.
LGTM
Just one comment I want to make sure we have checked about loading the config from the environment locally.
| await cloneRepoIntoTmpLocation({owner, repo}); | ||
|
|
||
| // Remove cached config and use the repo config. | ||
| // TODO(alanagius): this is needed because `setupConfigAndGitClient` uses a dummy config. |
There was a problem hiding this comment.
Have we verified this will work even if the config is defined using a typescript files? I know we have tried to do this in the past and found it didn't work in all locations with the changes we tried to make.
There was a problem hiding this comment.
I thought we migrate those, but looks like components are still using typescript configs.
We can either migrate that, or keep the dummy config if config is in typescript format.
There was a problem hiding this comment.
I'm fine with either, I think we just need to make sure that we don't get into a failing state with the change
There was a problem hiding this comment.
I went with not failing of the config is not found.
The branch-manager local GitHub Action previously relied on a mocked minimal configuration which lacked the "release" configuration block. As a result, validations checking the `TARGET_LTS` target label failed because they could not determine active LTS branches (which requires release config settings). This caused LTS PRs to throw an `InvalidTargetLabelError` and remain in a "pending" status permanently. This change fixes the issue by dynamically importing the actual repository's `.ng-dev/config.mjs` configuration after the repository is cloned inside the local action workspace, merging it with action input overrides and re-caching it. We also update `setCachedConfig` to support clearing/overwriting the cache by allowing `null` values.
|
This PR was merged into the repository. The changes were merged into the following branches:
|
2 participants
The branch-manager local GitHub Action previously relied on a mocked minimal configuration which lacked the "release" configuration block. As a result, validations checking the
TARGET_LTStarget label failed because they could not determine active LTS branches (which requires release config settings). This caused LTS PRs to throw anInvalidTargetLabelErrorand remain in a "pending" status permanently.This change fixes the issue by dynamically importing the actual repository's
.ng-dev/config.mjsconfiguration after the repository is cloned inside the local action workspace, merging it with action input overrides and re-caching it.We also update
setCachedConfigto support clearing/overwriting the cache by allowingnullvalues.