Add support for transitive NoWarn properties from project references #1672
Conversation
This reverts commit 41d1a6d.
mishra14
requested review from
rohit21agrawal,
PatoBeltran,
emgarten,
zhili1208 and
jainaashish
mishra14
changed the title
|
|
||
| var targetFrameworkInformation = packageSpec | ||
| .TargetFrameworks | ||
| .First(tfi => tfi.FrameworkName == framework); |
There was a problem hiding this comment.
Use packageSpec.GetTargetFramework, it should have this optimization already, and it will ensure that if a comparer is needed that we already have it
| { | ||
| get | ||
| { | ||
| if (_transitiveWarningPropertiesCollection == null) |
build/Shared/EqualityUtility.cs
Outdated
| /// Compares two sets for equality, allowing either sequence to be null. | ||
| /// If one is null, both have to be null for equality. | ||
| /// </summary> | ||
| internal static bool SetEqualWithNullCheck<T>( |
There was a problem hiding this comment.
Make equals plural to match the other methods
| // Add a constant hash for all objects | ||
| hashCode.AddObject(1); | ||
|
|
||
| return hashCode.CombinedHash; |
There was a problem hiding this comment.
Why not just return 1 if this isn't changing?
| private bool IsWarningSuppressed(IRestoreLogMessage message) | ||
| { | ||
|
|
||
| if (ProjectWarningPropertiesCollection != null && ProjectWarningPropertiesCollection.ApplyWarningProperties(message)) |
There was a problem hiding this comment.
nit: ProjectWarningPropertiesCollection?.ApplyWarningProperties(message) == true would simplify these
| using System.Collections.Concurrent; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using NuGet.DependencyResolver; |
|
NuGet.Commands.Test.StandaloneProjectTests.StandaloneProject_BasicRestore fails due to not having a project path set. Adding it to the test fixes it, you might also want to consider handling this gracefully. |
… not yet available to public (#1673)
mishra14
changed the title
| @@ -130,6 +130,8 @@ public void Log(IRestoreLogMessage message) | |||
| // This will be true if the message is not or warning or it is not suppressed. | |||
| if (!IsWarningSuppressed(message)) | |||
| { | |||
| UpgradeWarningIfNeeded(message); | |||
There was a problem hiding this comment.
add a comment explaining what this does
Porting over from : #1642
This branch contains a slightly different implementation with minimal warning property storage at each node in the graph.
Spec: https://github.com/NuGet/Home/wiki/%5BSpec%5D-Transitive-Warning-Properties
Fixes: NuGet/Home#5691 and NuGet/Home#5501
This change adds the capability to flow
NoWarnproperties transitively. The details are in the spec along with a high level implementation detail section.