Build: Update CircleCI configuration to remove build-sandboxes job #30360
Conversation
valentinpalkovic
added
ci:normal
build
|
View your CI Pipeline Execution ↗ for commit d5e903a.
☁️ Nx Cloud last updated this comment at |
valentinpalkovic
force-pushed
the
valentin/remove-build-sandboxes-step
branch
from
6cab86a to
544de57
Compare
…ild process; remove unnecessary logging in options utility
…djust working directory for event log checker
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
valentinpalkovic
added
ci:daily
valentinpalkovic
force-pushed
the
valentin/remove-build-sandboxes-step
branch
from
43857dc to
cd5fe5e
Compare
valentinpalkovic
added
ci:normal
and removed
ci:daily
valentinpalkovic
added
ci:daily
valentinpalkovic
added
ci:merged
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
17 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| TEMPLATE=$(yarn get-template --cadence << pipeline.parameters.workflow >> --task sandbox) | ||
| yarn task --task build --template $TEMPLATE --no-link --start-from=sandbox --junit | ||
| if [[ $TEMPLATE != bench/* ]]; then |
There was a problem hiding this comment.
logic: The build task is now run with --start-from=sandbox instead of the sandbox task with --start-from=never. Verify this change doesn't skip necessary build steps.
| yarnPath: ../.yarn/releases/yarn-4.3.0.cjs | ||
| # Sometimes you get a "The remote archive doesn't match the expected checksum" error, uncommenting this line will fix it | ||
| # checksumBehavior: 'update' | ||
| yarnPath: .yarn/releases/yarn-4.6.0.cjs |
There was a problem hiding this comment.
logic: The yarnPath is now relative to the current directory (.yarn/releases) instead of the parent directory (../.yarn/releases). Verify this path is correct and accessible in all build contexts.
| async function run({ template }: RunOptions) { | ||
| console.log(template.replace('/', '-')); |
There was a problem hiding this comment.
logic: template is marked as optional in RunOptions but not checked for undefined here. Could throw error if template is undefined.
| async function run({ template }: RunOptions) { | |
| console.log(template.replace('/', '-')); | |
| async function run({ template }: RunOptions) { | |
| if (!template) throw new Error('Template name is required'); | |
| console.log(template.replace('/', '-')); |
| type RunOptions = { | ||
| template?: string; | ||
| }; |
There was a problem hiding this comment.
style: template should be required since the script won't work without it
| type RunOptions = { | |
| template?: string; | |
| }; | |
| type RunOptions = { | |
| template: string; | |
| }; |
| if (esMain(import.meta.url)) { | ||
| program | ||
| .description('Retrieve the sandbox directory for template name') | ||
| .option('--template <template>', 'Template name'); |
There was a problem hiding this comment.
style: consider making --template required using .requiredOption() instead of .option()
| .option('--template <template>', 'Template name'); | |
| .requiredOption('--template <template>', 'Template name'); |
valentinpalkovic
added
the
patch:yes
Closes #
What I did
Workflow
dailybefore:Workflow
dailyafter:Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
Here's my summary of the key changes in this PR:
This PR optimizes CircleCI configuration by merging jobs and improving workspace persistence, while also updating core dependencies.
build-sandboxesintocreate-sandboxesjob, saving 3-5 minutes per workflownode_modules, reducing attach time by ~1:30 minutesget-sandbox-dir.tsscript to handle sandbox directory name generationThe changes show improved build times but introduce some concerning performance regressions in dev environment responsiveness that should be investigated.