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

Remove dnxcore50 moniker from lock file #2813

Merged
merged 2 commits into from Oct 2, 2017

Conversation

Projects
None yet
2 participants
@forki
Member

forki commented Oct 2, 2017

fixes #2810

@matthid

matthid approved these changes Oct 2, 2017

Schaut gut aus. ich frage mich ob man den Filter auch in extractPlatforms reinziehen kann. Aber vermutlich eher nicht weil sonst Gruppen als Fallback Gruppen erkannt werden

@@ -176,7 +179,8 @@ let tryGetDetailsFromCache force nugetURL (packageName:PackageName) (version:Sem
if (PackageName cachedObject.PackageName <> packageName) ||
(cachedObject.Version <> version.Normalize())
then
traceVerbose (sprintf "Invalidating Cache '%s:%s' <> '%s:%s'" cachedObject.PackageName cachedObject.Version packageName.Name (version.Normalize()))
if verbose then

This comment has been minimized.

@matthid

matthid Oct 2, 2017

Member

macht den check nicht auch traceVerbose schon?

@matthid

matthid Oct 2, 2017

Member

macht den check nicht auch traceVerbose schon?

This comment has been minimized.

@forki

forki Oct 2, 2017

Member

yes, but the right side (sprintf ...) would be evaluated even if verbose is false.
This was serious perf problem in the past

@forki

forki Oct 2, 2017

Member

yes, but the right side (sprintf ...) would be evaluated even if verbose is false.
This was serious perf problem in the past

This comment has been minimized.

@matthid

matthid Oct 2, 2017

Member

Should we remove traceVerbose in that case ?

@matthid

matthid Oct 2, 2017

Member

Should we remove traceVerbose in that case ?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Oct 2, 2017

Member

if we move it into extractPlatforms then we get the annoying warnings that dnxcore is not real moniker

Member

forki commented Oct 2, 2017

if we move it into extractPlatforms then we get the annoying warnings that dnxcore is not real moniker

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Oct 2, 2017

Member
Member

forki commented Oct 2, 2017

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Oct 2, 2017

Member

Tests need to be adapted. The current failure looks a bit suspicious, but it might be that the error message is just incomplete and everything is fine.

Member

matthid commented Oct 2, 2017

Tests need to be adapted. The current failure looks a bit suspicious, but it might be that the error message is just incomplete and everything is fine.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Oct 2, 2017

Member
Member

forki commented Oct 2, 2017

@forki forki merged commit 7e4acae into master Oct 2, 2017

0 of 4 checks passed

continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@forki forki deleted the i2810 branch Oct 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment