fix(yarn2): only set immutable cache if cache available #832
Conversation
lib/dependencies.sh
Outdated
| @@ -109,6 +109,11 @@ yarn_2_install() { | |||
|
|
|||
| echo "Running 'yarn install' with yarn.lock" | |||
| cd "$build_dir" || return | |||
|
|
|||
| if ! has_yarn_cache "$build_dir"; then | |||
| echo "Yarn cache is required. See https://devcenter.heroku.com/articles/migrating-to-yarn-2#make-changes-to-source-code for additional information" | |||
There was a problem hiding this comment.
What do you think about changing this to:
if ! has_yarn_cache "$build_dir"; then
monitor "yarn-2-install-with-node-modules" yarn install 2>&1
else
...There was a problem hiding this comment.
that's what i thought of doing originally but i didn't want to be presumption with asking your team first haha.
mlavina
force-pushed
the
mlavina/feat/heroku/immutableCache
branch
from
5dcdaac
to
6895115
Compare
|
@danielleadams any ideas why the hatchet stuff is failing. I tested locally with make heroku-20. I can try a full make test but that's going to take a bit on my laptop |
mlavina
changed the title
mlavina
changed the title
|
QQ @lillianzhang331 will #833 solve the hatchet issue here. I am sorry, but I don't really understand what the hatchet test is and why it's failing in my PR |
mlavina
force-pushed
the
mlavina/feat/heroku/immutableCache
branch
2 times, most recently
from
23e86b3
to
4a2bd87
Compare
|
@lillianzhang331 I ran Do I have to register my username somewhere? |
|
@mlavina Hatchet tests are only supposed to run when members of Heroku team open pull requests (and when we create tests to check release acceptance). cc @lillianzhang331 We may want to check why the Hatchet tests are running when a contributors open PRs |
|
(Not sure if it was always like that, but |
|
@mlavina do you mind rebasing this? I am back from vacation, so I can work on getting this in this week. |
mlavina
force-pushed
the
mlavina/feat/heroku/immutableCache
branch
from
4a2bd87
to
9c9030e
Compare
|
@danielleadams rebased |
|
@mlavina Going to fix these tests and then we can get it merged. Thanks again for the work! |
|
There's a configuration the CI isn't picking up. I'm going to try closing the pull request and reopening to see if it picks it up. |
|
@mlavina For some reason the Travis build is not picking up the environment variables - I think this is because the pull request was created before the variables were added. Do you mind creating a new pull request? Sorry for the additional work. No need to write an explanation - you can just link this PR because we'll want to reference the discussion here anyways. Thanks! |
|
sorry this is being so confusing i opened up #836 |
I ran into an issue where I didn't have cache committed into source because I was using node-linker and didn't think i needed that.
I added this first commit to warn about that if someone else runs into that issue
But i was going to add a second commit to disable immutable cache if you are using node-linker
What are your thoughts @danielleadams @lillianzhang331 ?
Should we disable immutable-cache for all node-linkers? Should there be a different opt-out param similar to yarn_production? Should there be an opt-out at all?