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

Missing documentation for process.env.CI #7344

Open
sguillia opened this issue Jul 10, 2019 · 2 comments
Open

Missing documentation for process.env.CI #7344

sguillia opened this issue Jul 10, 2019 · 2 comments

Comments

@sguillia
Copy link

The npm run build command is affected by the environment variable CI, but it is not well documented.

This is what the existing documentation says about it:

When creating a build of your application with npm run build linter warnings are not checked by default. Like npm test, you can force the build to perform a linter warning check by setting the environment variable CI. If any warnings are encountered then the build fails.

By looking at the source code or playing with the command, there are several side effects:

I believe it would be a good choice to have no undocumented side effect.


Use cases where it is a problem:

  • I want to master my CI pipeline: I can't just set an environment variable without knowing exactly what it does.
  • I want to treat warnings as errors locally and the only option seems to be to set the CI option (this is a problem in intslef): I would also like to know what it does and keep the colors in the output.
  • The usage of npm build don't talk about the envinroment variables, neither does npm help build, it is then confusing to see the behavior change between computers.
$ npm build -h  
npm build [<folder>]

Thank you for reading this.

Best,
Simon

@mcqj
Copy link

mcqj commented Jun 18, 2020

There is a good reason why linting rules separate warning and errors. Warnings are intended to alert the developer to take a look at something. The developer then makes a judgement call on whether there is a real issue. I don't want tooling to make that decision for me. If there is an option that can be set to treat warning as errors that could be very useful and many people might use it. However, it should be an option that is not combined with other behavioural modifications in the build.

@christiannaths
Copy link

christiannaths commented Oct 1, 2020

I see half a dozen issues with developers asking why this would be a default setting (CI=true means warnings are treated as errors). It's bizarre. These other issues are closed and locked without explanation—as I'm sure this one will be too—but instead are brushed off with a "we know best" type of answer.

If my team decides that some lint violation is acceptable as a warning instead of an error, and we make that change to our config, I very much expect any CI to abide by that. Setting process.env.CI=false does appeal to me at all. It's a blackbox flag: I have no idea what it does now, or what it might do in the future. The environment is a CI, why on earth would I turn that setting off?

I appeal to @gaearon and @Timer to stop shutting this down, and either shed a light on the internal discussions y'all had surrounding this, or listen to the community. Over the years of working with CRA I've started to find this kind of cold shoulder (coldly shutting down issues and locking discussions) really off-putting. I apologize for my tone, I respect the work that's been done, and I realize you owe me nor any of us anything, but it's frustrating none-the-less. 😕

Edit: I'm commenting here because all of the directly-related issues have been locked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants