fix: don't read envs when explicitly not in CI

[karl-run]

Should fix #139.

[karl-run]

Diff looks a bit icky, but it's literally just wrapping the vendors.forEach with if (env.CI !== 'false') {.

[karl-run]

Tested with

CI=false deno run --allow-env=CI --allow-read=./ index.js 

Works as expected.

[sibiraj-s]

Thanks @karl-run

[wraithgar]

This may have constituted a breaking change btw. We're still investigating but it broke CI tests in npm's publishing library.

ETA: All good. We had a mocked environment that was setting CI to false but GITHUB_ACTIONS to true, which technically invalid.

[sibiraj-s]

@wraithgar . Apologies, Yes its indeed looks like a breaking change, since the vendor constant is set now regardless of CI true/false until v4.3.0.

Though it was the expected, given the no. of users and not sure how the vendor constants are being used currently, I am considering publishing a patch reverting this change.

And may be put this in v5 may be. Open to thoughts.

[wraithgar]

Open to thoughts.

This is an EXTREMELY grey area. What does env.CI=false mean? There is no mention of it in the README so we can't go off of that.

The code says:

// Bypass all checks if CI env is explicitly set to 'false'

But this ONLY applies to isCI. Does it make sense to also set vendor envs here? I think the change does make sense. If I have explicitly set CI to false that's a pretty explicit state I've set.

As one of the developers of npm I am no stranger to users relying on bugs, and when we fix those bugs folks have something break. It's a frustrating experience for everyone.

I would tend towards leaving the change, and adding language to the README.md to avoid confusion in the future.

[sibiraj-s]

My bad, Missed the comment.

Agreed. And yes, will add this behavior to README.