-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Warnings are being swallowed (unless --error-on-warnings
is on)
#535
Comments
Yeah, I think passing STDERR through like STDOUT makes sense. |
not as stderr? I was planning on forwarding it to stderr |
Yeah, pass STDERR through to STDERR of course. I just compared it to STDOUT because there we already do that. |
@beta-ziliani this means the raise exception should change? Line 48 in 1739c0f
Now STDERR is already part of the output so there is no Is that what you mean? If that is the case, something like the following is what you would be expecting the output to be?
And when it errors, the last line will be Thank you |
Sorry for the delay, @luislavena , yes, that's what I mean |
No worries @beta-ziliani, thanks for responding! I will be happy to send a PR with this change later this week 😊 Cheers. PS: Happy New Year! 🎉 |
I'm finishing on one right now :-) But I'll be happy to have your review. Happy new year for you too! 🎉 |
Solves #535 by outputting the STDERR of the crystal build to the STDERR of the shards process only if the build is successful. This allows to notice deprecation warnings and the like.
Thank you @beta-ziliani!!! 🎉 🥳 👏🏽 I think this one can be closed now that #540 has been merged! |
Basically, the problematic line is:
shards/src/commands/build.cr
Line 47 in 1739c0f
It is writing the error on a local memory object that is discarded unless there is an unsuccessful build.
Example:
➜ d git:(main) ✗ cat shard.yml name: d version: 1 targets: d: main: d.cr ➜ d git:(main) ✗ cat d.cr @[Deprecated] def a end a ➜ d git:(main) ✗ shards build Dependencies are satisfied Building: d ➜ d git:(main) ✗ crystal build d.cr In d.cr:5:1 5 | a ^ Warning: Deprecated top-level a. A total of 1 warnings were found.
The text was updated successfully, but these errors were encountered: