New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: avoid unnecessary release builds #5191
base: master
Are you sure you want to change the base?
ci: avoid unnecessary release builds #5191
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
@@ Coverage Diff @@
## master #5191 +/- ##
=======================================
Coverage 98.86% 98.86%
=======================================
Files 230 230
Lines 8805 8805
Branches 2314 2314
=======================================
Hits 8705 8705
Misses 39 39
Partials 61 61 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this not just result in us publishing dev builds because the artifacts are not built during the publish
job but during the build
job where the ROLLUP_RELEASE
variable is not set?
Is there a scenario where building and releasing occur outside the CI environment? |
Publishing will only happen on CI, but building does not happen during the publish phase and does not have the environment variable set. So you would need to set it conditionally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks really interesting. I still think, though, that this would publish dev builds because the env: ROLLUP_RELEASE: 'releasing'
is only set during the publish
job and not during the build
job.
I would expect something like
env:
BUILD_ARTEFACTS: "${{ startsWith(github.ref_name, 'v4') && 'build' || 'dev' }}"
added to the build
job, which would use the same condition as the publish
job.
Also, maybe BUILD_ARTEFACTS: "release"
would be less confusing than "build"
because we are building them in any case, but we either build the "release" or the "dev" version
@@ -311,6 +311,7 @@ jobs: | |||
needs: | |||
- test | |||
- lint | |||
- build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As test needs build, this should not be needed (but of course does not hurt, either)
target: ${target} | ||
`); | ||
|
||
exec(commandToRun, (error, stdout, stderr) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using exec
, there is a nice runWithEcho
helper in scripts/helpers.js
that has the advantage that it prints output while the command is running instead of waiting for the end. You would need to turn the arguments into an array, though, but it should make for a better experience when developing locally.
Also, it promisifies the output, which means instead of a force exit, you can just top-level await the function so that errors will abort the script automatically with the full error.
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
Non-release environments will not trigger a release build and will use a dev build instead, reducing build time.