Frameworks within the portable profile are not allowed to have profiles themselves #2823
Conversation
|
/cc @emgarten |
maartenba
force-pushed
the
issue-2822
branch
from
c2589ca
to
a23ab9d
Compare
maartenba
force-pushed
the
issue-2822
branch
from
241b9f5
to
bfca1e5
Compare
| @@ -569,6 +583,24 @@ private static void ValidateNuGetPackageMetadata(PackageMetadata packageMetadata | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| private static void ValidateSupportedFrameworks(string[] supportedFrameworks) | |||
| { | |||
There was a problem hiding this comment.
can we instead of explicitly checking for the hyphen, share the code that the indexing task is using?
There was a problem hiding this comment.
Indexing uses NuGet.Core still (which does the check similar to this)
There was a problem hiding this comment.
So should we move this check into the client code, and file a bug to have indexing move off nuget.core?
There was a problem hiding this comment.
Should be replicated in client ( and then removed here).
V2v3 is the new indexing code that also drops nuget.core, so should automatically benefit from client fix.
Can I deploy this one to gallery for the time being?
There was a problem hiding this comment.
this looks good to me, I'd suggest another signoff, and I would suggest that you start the client fix/indexing code (or @joyhui can suggest someone else)
There was a problem hiding this comment.
I am fine with this change. But since we plan to add bunch of validation in our road map, we may refine the validation structure anyway.
faster?
if( supportedFrameworks.Any( fx => !string.IsNullOrEmpty(fx) && fx.StartsWith("portable-", StringComparison.OrdinalIgnoreCase) && fx.Split('-').Length > 2)))
{
throw ...
}
There was a problem hiding this comment.
Created client issue: NuGet/Home#1869
maartenba
force-pushed
the
issue-2822
branch
2 times, most recently
from
5643e37
to
114b34b
Compare
| } | ||
|
|
||
| var shortFolderName = frameworkName.GetShortFolderName(); | ||
| if (String.Equals(shortFolderName, "any", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
Same comments as above.
If we have same help class across project, we may want to extract them to a common project or may be add helper class as reference to the project.
|
I am ok with the change. |
maartenba
force-pushed
the
issue-2822
branch
from
114b34b
to
b0eb555
Compare
Frameworks within the portable profile are not allowed to have profiles themselves
Fixes #2822