ci: combine "define_env_vars" and "download_yarn" anchor #28788
Conversation
devversion
force-pushed
the
ci/cleanup-circleci-config
branch
3 times, most recently
from
d4399da
to
cd147ea
Compare
devversion
added
area: build & ci
devversion
added
the
action: review
devversion
force-pushed
the
ci/cleanup-circleci-config
branch
from
cd147ea
to
a0dcb58
Compare
| command: ./.circleci/env.sh | ||
| name: Initializing environment (setting up variables, downloading Yarn) | ||
| command: | | ||
| source ./.circleci/env.sh |
There was a problem hiding this comment.
Why do you need to source it now?
There was a problem hiding this comment.
It's because we need the yarn version variable available in the current bash shell. The command after the env.sh script depends on it.
Not sure if we want to add a comment for this. Should be quite clear IMO 😁
There was a problem hiding this comment.
I thought that PROFILE=$BASH_ENV was sufficient. Fair enough then 😏
There was a problem hiding this comment.
😄 Yeah that PROFILE variable is just used by the Yarn install script to update the $PATH.
There was a problem hiding this comment.
We've gone back and forth between installing tooling during the run (slower) or building the right environment into the docker image (more maintenance)
I feel like the latter is the more principled thing but maybe not practical, up to you
With more work, we ought to be able to have both:
- change the Dockerfile, the CI picks that up and builds a new docker image for this build
- don't need to do any environment setup on each execution
There was a problem hiding this comment.
Yeah I'm personally a bigger fan of preparing a proper docker image. We should have a discussion about this in a follow-up as this should be something we need to figure out long-term.
As discussed in angular#28546 (comment), we want to combine the `define_env_vars` and `download_yarn` anchor since downloading Yarn depends on setting up the environment variables. In addition this simplifies our setup and reduces code-duplication.
devversion
force-pushed
the
ci/cleanup-circleci-config
branch
from
a0dcb58
to
f577341
Compare
devversion
added
action: merge
As discussed in #28546 (comment), we want to combine the `define_env_vars` and `download_yarn` anchor since downloading Yarn depends on setting up the environment variables. In addition this simplifies our setup and reduces code-duplication. PR Close #28788
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
As discussed in #28546 (comment), we want to combine the
define_env_varsanddownload_yarnanchor since downloading Yarn depends on setting up the environmentvariables. In addition this simplifies our setup and reduces code-duplication.