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

Revisit requireFail color and message #142

Closed
phated opened this issue Jan 2, 2018 · 10 comments
Closed

Revisit requireFail color and message #142

phated opened this issue Jan 2, 2018 · 10 comments

Comments

@phated
Copy link
Member

phated commented Jan 2, 2018

It seems that some consumers are allergic to the color red - we should revisit the coloring and messaging of the requireFail log.

Ref foundation/foundation-sites#10828
and gulpjs/gulp#1631

@ncoden
Copy link

ncoden commented Jan 2, 2018

@phated I am really impressed by how close you are to the gulp community. Thank you and 👍 for this issue.

@phated
Copy link
Member Author

phated commented Jan 23, 2018

We might also want to change this to a log.warn

@njnygaard
Copy link

I'm allergic to the color red because it gets passed through to my docker build. It is not a failure, just attention grabbing. Just as a note, npm warnings about "EINTEGRITY" come through as 'red' too in a docker build. And what I mean by that is log.warn might not quiet those with the allergy.

@phated
Copy link
Member Author

phated commented Jan 25, 2018

@sttk I just realized this issue wasn't fully completed by my PR. We still need to change the wording of the messages. I also think a "beforeRequire" event in liftoff would help improve messaging. Thoughts?

@phated
Copy link
Member Author

phated commented Jan 25, 2018

@njnygaard I'm guessing you don't know of our --no-color flag?

@phated phated reopened this Jan 25, 2018
@yocontra
Copy link
Member

Why are we logging this message at all if it ends up working?

@phated
Copy link
Member Author

phated commented Jan 25, 2018

@contra some of the logs happen in realtime (require logged right after it's required or fails) while some is batched but we don't know which is which. Also, I think there was a desire to show the user when they were using an older loader (e.g. babel-register instead of @babel/register)

@sttk
Copy link
Contributor

sttk commented Jan 28, 2018

@phated

I also think a "beforeRequire" event in liftoff would help improve messaging.

Yes, I remember that there is a request to add the event to liftoff (gulpjs/liftoff#53). I agree it, and I'll address it if I can.

@Saturate
Copy link

Saturate commented Apr 4, 2018

This message has caused some trouble with some CI servers for us, just bailing out on the build due to red or the words failed.

We could just update the babel setup, but don't feel like updating to @babel/register as it's still in beta, the latest version is 7.0.0-beta.44

@phated
Copy link
Member Author

phated commented Mar 13, 2019

This was closed by #182 because it pulled in Liftoff 3.1, and v3 removed the extra logging if we found 1 successful loader.

It should be released very soon.

@phated phated closed this as completed Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants