Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add cloud recommendation message to CI output #24680
feat: add cloud recommendation message to CI output #24680
Changes from 23 commits
3d90508
dda9324
5b3af89
6649647
9781e3f
3faaa2a
166a994
12fa855
606fcec
396c650
b067b6a
4bc9b36
2a065a1
347f73e
a5a44e3
c71faec
55ec51b
b56b026
f4df2c5
3934092
0500820
1768a3b
4fd58ef
5fbabd4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why do these need to be in the force-no-rewrite category?
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'm trying to figure out exactly why, but it wouldn't work without adding those (they probably don't all need to be in there).
I think what might be happening is that the exported value from
is-ci
is getting evaluated and snapshotted beforeprocess.env.CI=true
is set, and then it never gets re-evaluated?If I don't have these in this file then
is-ci
returnsfalse
even though we are running in CI and it should betrue
. @ZachJW34 originally suggested trying this when we were working through itThere 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.
@astone123 and I paired on this a bit. Not sure we need every one of these but it wasn't working without this addition. I believe it's due to how
is-ci
andci-info
are written, where the export is evaluated upon require e.g.module.exports = !!process.env.CI
and not a function.Could be totally wrong though!
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 from playing around locally all we need to add here is the first two and I'm good with that for now. Though I still want to figure a little more on why.