Warnings are being swallowed (unless `--error-on-warnings` is on)

[beta-ziliani]

Basically, the problematic line is: https://github.com/crystal-lang/shards/blob/1739c0fb96cf16e405bcdebf91357540d87243b3/src/commands/build.cr#L47

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.

[straight-shoota]

Yeah, I think passing STDERR through like STDOUT makes sense. shards build is supposed to be just a thin wrapper around crystal build.

[beta-ziliani]

not as stderr? I was planning on forwarding it to stderr

[straight-shoota]

Yeah, pass STDERR through to STDERR of course. I just compared it to STDOUT because there we already do that.

[luislavena]

@beta-ziliani this means the raise exception should change?

https://github.com/crystal-lang/shards/blob/1739c0fb96cf16e405bcdebf91357540d87243b3/src/commands/build.cr#L48

Now STDERR is already part of the output so there is no error to use for that message.

Is that what you mean?

If that is the case, something like the following is what you would be expecting the output to be?

$ shards build
Dependencies are satisfied
Building: d
In d.cr:5:1

 5 | a
     ^
Warning: Deprecated top-level a.

A total of 1 warnings were found.

And when it errors, the last line will be Error target d failed to compile. with the entire error already shown above (due the usage of STDERR)

Thank you ❤️ ❤️ ❤️

[beta-ziliani]

Sorry for the delay, @luislavena , yes, that's what I mean

[luislavena]

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! 🎉

[beta-ziliani]

I'm finishing on one right now :-) But I'll be happy to have your review. Happy new year for you too! 🎉

[luislavena]

Thank you @beta-ziliani!!! 🎉 🥳 👏🏽

I think this one can be closed now that #540 has been merged! ❤️ ❤️ ❤️