Skip to content
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(core): improve checks for process.env.CI #22694

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Conversation

luckened
Copy link
Contributor

@luckened luckened commented Apr 5, 2024

Current Behavior

Today, nx does not disable the output styling and WoodpeckerCI doesn't go well with it, This requires us to manually add "output-style=stream" to every nx command, which solves the issue but I understand that we could have a woodpecker check to isCI function as well

reference:
https://woodpecker-ci.org/docs/usage/environment#built-in-environment-variables

Expected Behavior

Disable styling in nx command outputs when running in WoodpeckerCI

Related Issue(s)

I haven't found a related issue, but this PR came as a suggestion in this discord thread: https://discord.com/channels/1143497901675401286/1225808236104646706

@ edit

the title is "feat(core): add WoodpeckerCI to isCI check" but the check also applies to other CI vendors that set process.env.CI to a non-empty string

@ edit2

I've improved the PR title with feat(core): improve checks for process.env.CI since it now covers all CI players that dont'set process.env.CI directly to "true" but to other values instead

@luckened luckened requested a review from a team as a code owner April 5, 2024 14:47
@luckened luckened requested a review from Cammisuli April 5, 2024 14:47
Copy link

vercel bot commented Apr 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Apr 12, 2024 3:59pm

Copy link

nx-cloud bot commented Apr 5, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f063626. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@luckened luckened changed the title chore: add woodpeckerCI to isCI() check feat(isCI): add WoodpeckerCI Apr 5, 2024
@luckened luckened changed the title feat(isCI): add WoodpeckerCI feat(core): add WoodpeckerCI to isCI check Apr 5, 2024
Copy link
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I think we can do this in a bit more agnostic way.

Rather than adding an additional check for woodpecker specifically, can you update the earlier check from process.env.CI === 'true' to (process.env.CI && process.env.CI !== 'false')

@luckened luckened force-pushed the patch-1 branch 2 times, most recently from a41b94a to 23d2f26 Compare April 5, 2024 16:15
@luckened
Copy link
Contributor Author

luckened commented Apr 5, 2024

@AgentEnder thank you for the review!

I see, I'm thinking if its okay to just check if process.env.CI exists, as I believe process.env.CI is not "false" locally and there may be other CI systems that set the variable to their own name, does this make sense?

done 👍

@luckened luckened force-pushed the patch-1 branch 6 times, most recently from 22106e3 to 34688ae Compare April 8, 2024 14:56
@luckened luckened requested a review from AgentEnder April 8, 2024 14:56
@luckened luckened changed the title feat(core): add WoodpeckerCI to isCI check feat(core): improve checks for process.env.CI Apr 8, 2024
@luckened luckened force-pushed the patch-1 branch 4 times, most recently from ab38675 to 851acc5 Compare April 9, 2024 18:07
@luckened luckened force-pushed the patch-1 branch 5 times, most recently from 23836b1 to a3b0874 Compare April 12, 2024 12:56
@AgentEnder AgentEnder merged commit 134cbbc into nrwl:master Apr 12, 2024
6 checks passed
@luckened luckened deleted the patch-1 branch April 15, 2024 16:49
AgentEnder pushed a commit to AgentEnder/nx that referenced this pull request Apr 23, 2024
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants