-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
chore: Move console overrides to reporter.log #14834
Comments
I removed the comment about this being blocked. I updated the issue so it's not blocking anymore. |
I'm going to use this one in my pairing session with someone. Sorry, it's taken ^^ |
This is back for the taking since the pairing session was a no show 😱 |
I'm happy to go through and refactor if it's still up for grabs? |
yes! All yours! |
Just to clarify, you want all of the console logs in the file above to be refactored as ...reporter.log(util.format(...args))? |
Yeah, all of them (console.log, console.warn, console.info and console.error) should call reporter.log(util.format(...args)) |
@sidharthachatterjee thanks, I'll start work on this tonight. |
Anyone about to quickly review my PR? @wardpeet @sidharthachatterjee |
@sidharthachatterjee Can I work on this, I see there are still |
@sidharthachatterjee is there something remaining in the issue? |
@kushthedude I've reverted #17838 In #14973, we noticed that the overrides would cause any logs via Incorrect reporting of levels is unfortunately pretty bad and I would consider the benefit of fixing that versus the problem that this issue describes a reasonable tradeoff. For now, I'll be closing this and will open an issue upstream in https://github.com/yeoman/update-notifier and see if we can fix it there! 🙂 Edit: Opened yeoman/update-notifier#170 |
We currently override
console.log
,console.warn
,console.info
andconsole.error
ingatsby/packages/gatsby-cli/src/reporter/index.js
Line 109 in e034605
This can get a little messy sometimes
In this case, the log from
update-notifier
shows up as an error.update-notifier
correctly calls console.error (because they want to log to stderr) but this isn't really an error for us and shouldn't show up as one (and potentially confuse some users)yeoman/update-notifier@c3b1df6#diff-168726dbe96b3ce427e7fedce31bb0bc
We want to move our console overrides to reporter.log. Console messages will still show pretty in ink but won't have a prefix (success, error, warn, info). We want to move towards a future where all gatsby stuff uses the reported directly.
The text was updated successfully, but these errors were encountered: