Repository navigation
Option not to fail build on warnings in CI environment #2453
Description
Currently if CI environment variable is set to true, and any warnings emitted, build fails (#944). There is no way to alter this behavior other than unsetting CI envvar that may be undesirable. You cannot even customize the rule set (#808).
Please add command-line option like --no-fail-on-warnings or add any other way to make this less strict.
Activity
gaearon commentedon Jun 2, 2017
What warnings specifically do you want to ignore?
varnav commentedon Jun 2, 2017
We are at the early stages of development of UI, and warning that annoys us most is 'jsx-a11y/alt-text' about no alt inside img. We intentionally don't have these for now.
But I would prefer just to have control over ESlint ruleset.
Timer commentedon Jun 6, 2017
@varnav you can disable eslint warnings by adding the lines suggested to you in your development console while running
npm start.varnav commentedon Jun 6, 2017
I don't run
npm startduring CI deployment process, and I don't want to disable all warnings. I want:Timer commentedon Jun 6, 2017
You can place
//eslint-disable jsx-a11y/alt-text(or similar) at the top of the file to only disable that warning (and not all warnings). This has the effect in development and production builds, I was simply referring to development as it provides instructions for you to do so.varnav commentedon Jun 7, 2017
It works as workaround, but I will have to change about 40 files. It would be better to have centralized control over this.
Timer commentedon Jun 7, 2017
Unfortunately you cannot have control of this unless if you eject; being mindful of accessibility when you are creating images doesn't take very long.
Is there something wrong with adding
alt="TODO"when you create an image? This makes it very easy later on to find where you need to fix these problems, rather than ignore it all together.If you explicitly would like to not provide an alt,
alt=""will mute the warning and is also relatively searchable.varnav commentedon Jun 7, 2017
You probably right, but in reality it looks different. I don't write js, I'm admin and I do DevOps things. And I have chats like this:
Programmers: Hey, build of our code fails in your CI!
Me: It's because you have warnings
P: But backend guys have warnings too, their build is ok. What can we do?
M: Fix warnings
P: We have tasks, we have deadlines, we can't do this, it's 250 lines in 40 files!
M: Ok, then add this to mute warnings to your files
P: All 40 files? We have tasks, we have deadlines. We gonna complain to your supervisor!
M: Sorry guys, I don't have control over fails/not fails on warnings, and over eslint ruleset too!
P: Yes, it's exactly what we gonna tell your supervisor. You don't have control over CI builds that are your direct responsibility!
Timer commentedon Jun 7, 2017
FWIW this has been discussed before and warnings purposely fail CI builds by default. This is good practice.
Disabling this functionality is normally done by setting
CI=false(as you mentioned in your first post); why is this not a viable option?To clarify, setting
CI=falseis already the official solution to this / the option you are looking for.If you can elaborate why this option is not good enough for you, we are more than happy to consider adding another solution.
varnav commentedon Jun 7, 2017
This is strange, because we have 5 more different languages built in CI, and nowhere else build fails on warnings.
Setting CI to false does not help, only unsetting does. But is this a good idea? Is this variable used somewhere else?
Timer commentedon Jun 7, 2017
If this is the case, then this is a bug. Setting
CIto befalseshould disable warnings.As described in Advanced Configuration, setting
CIonly makesnpm testnon-watching and makesnpm run buildwarnings failures. There are no other side-effects, so this is completely safe to do.Make sure you're only doing this for the
npm run buildcommand, however. You probably should not unset it in your entire test/CI environment.Timer commentedon Jun 7, 2017
It seems that setting
CIto false will indeed still treat the variable as true.As a temporary work-around, you can set CI to be blank via
CI="" npm run buildin your test script.We're happy to accept a PR that looks for the value
'false'.11 remaining items
varnav commentedon Jun 27, 2017
I don't really think that it's ok to close this issue, as "Option not to fail build on warnings in CI environment" still does not exist.
gaearon commentedon Jun 28, 2017
I don't think we'll be adding an option for this. But we're happy to address individual warnings that "shouldn't be warnings" on a case by case basis. If you see an unnecessary warning please file an issue.
Check if CI variable is set to `'false'` (react#2501)
Check if CI variable is set to `'false'` (#2501)
Unset $CI so the warnings != errors when building
Unset $CI so the warnings != errors when building
bompi88 commentedon Dec 5, 2017
This is not a preferable solution when a lot of frameworks and libraries depend on the CI variable in different ways. My build now fails because of named imports, which are not in alphabetical order.
Timer commentedon Dec 5, 2017
We do not enforce a rule about alphabetical ordering -- it sounds like you have your own rules in place.
If you have an problem, please file a new issue. Thanks!
removed warnings, facebook should fix react/create-react-app#2453
removed more warnings, facebook should really fix react/create-react-…
Revert "removed more warnings, facebook should really fix react/creat…
Revert "removed warnings, facebook should fix react/create-react-app#…