GH1073: dotnet CLI preview 2 doesn't have the quiet option anymore #1016
Conversation
|
Hi @adamhathcock, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
|
Looks like the builds are failing because your build script on the dotnet cli also uses the Quiet option. |
|
@adamhathcock Great catching this! The reason for the builds failing is that the tests for DotNetRestore has not been updated. |
|
I'll update |
adamhathcock
force-pushed
the
remove_quiet_restore
branch
from
8418b20 to
6966152
Compare
|
Now it's failing because of Warning As Error. Should I just remove the obsolete attribute? |
|
@adamhathcock Yes. I think you can remove that attribute for now and just write to the log in the Tool (as a warning) that the flag has been removed from dotnet cli RTM and they should set verbosity instead. |
|
Isn't this just that the tests should pragma ignore the obsolete attribute? |
|
@devlead @adamhathcock Yes, that would be preferred. |
| @@ -38,7 +40,8 @@ public sealed class DotNetCoreRestoreSettings : DotNetCoreSettings | |||
|
|
|||
| /// <summary> | |||
| /// Gets or sets a value indicating whether to display any output. | |||
| /// </summary> | |||
| /// </summary> | |||
| [Obsolete("dotnet cli does not have this option")] | |||
There was a problem hiding this comment.
Can we change this to .NET CLI does not support this option anymore. Please use DotNetCoreRestoreSettings.Verbosity instead.
adamhathcock
force-pushed
the
remove_quiet_restore
branch
3 times, most recently
from
9a83121 to
32c6e03
Compare
|
finally did the pragmas in all the correct places. |
| { | ||
| builder.Append("--quiet"); | ||
| settings.Verbosity = DotNetCoreRestoreVerbosity.Minimal; |
There was a problem hiding this comment.
I don't think we should set this propery automatically. Better to write to the log as a warning giving the same hint as the obsolete message.
|
@adamhathcock are you in a position to update with the comment that was made by @patriksvensson |
|
I've been on vacation for the past two weeks. I can fix it on Wednesday |
|
Looking at this now: I need a Making the tool take the context involves changing the tests a lot at least. |
adamhathcock
force-pushed
the
remove_quiet_restore
branch
from
32c6e03 to
08f44d7
Compare
|
Nevermind, I was being dumb. I think I've done everything requested....but now I just noticed I have to update the branch again |
adamhathcock
force-pushed
the
remove_quiet_restore
branch
from
08f44d7 to
790a0f2
Compare
|
@gep13 @patriksvensson finally updated and passed. Needs a small review again |
adamhathcock
force-pushed
the
remove_quiet_restore
branch
from
790a0f2 to
545ce71
Compare
|
@gep13 should I create an issue? |
|
@adamhathcock yes, if you could. Ideally, an issue is created, which gives us a place to discuss the problem/feature before any work is undertaken, and then we use the issue title to automatically generate the release notes for each release. |
|
I thought it was a simple fix so I didn't...next time I promise I will :) |
Not a problem at all. For anything that is a documentation change, we typically don't want/need issues created, however, anything that is a code change, we would like to see an issue getting raised/discussed first, just in case something is already being worked on, or not required, etc. The last thing we want is people doing work that isn't required, and then having to close the PR. Obviously, that won't be the case here, but we like to follow the same process, so that there is some consistency, and people know what to expect. |
|
Relates to #1073 |
|
LGTM! Thanks again for the contribution. |
gep13
changed the title
Just noticed this.