chore: code review feedback for #3195

[alexeagle]

cc @sluongng

[sluongng]

There is a small logic flaw I spot. The rest looks good to me.

[sluongng]

This unfortunately will add --cache-folder for folks who use Yarn Berry and set user_global_yarn_cache to False.

The flag DOES exist within Yarn Berry, but the behavior is to exit with a deprecation error which is not useful:

https://sourcegraph.com/github.com/yarnpkg/berry/-/blob/packages/plugin-essentials/sources/commands/install.ts?L207-219
https://sourcegraph.com/github.com/yarnpkg/berry@ce1e0c31781e3fc791cfa86a01348c4f0dba9aec/-/blob/packages/gatsby/content/advanced/error-codes.md?L295-301

I suggest ensure that refactor it like this:

 if yarn_version == "classic":
    if repository_ctx.attr.use_global_yarn_cache:
        ...
    else:
        ...

if yarn_version == "berry" and not repository_ctx.attr.use_global_yarn_cache:
    # Using Yarn Berry and non-default option for global_yarn_cache
    fail("Option global_yarn_cache has no effect while using Yarn Berry. Please configure it through a .yarnrc file through 'data' attribute.")

[alexeagle]

yeah my intent here was the minimal change. thanks for looking up the yarn sources, if that flag just exits that way then I agree we shouldn't call it

[sluongng]

Thanks. This revision looks good to me 👍