这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@rchande
Copy link

@rchande rchande commented Sep 9, 2019

No description provided.

events
.First()
.Should()
.Match(e => e is DisplayedValueProduced && ((DisplayedValueProduced)e).Value.ToString().Contains("Attempting to install"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably here we should assert on a sequence of events

if (!string.IsNullOrWhiteSpace(package.PackageVersion))
{
message += $", version {package.PackageVersion}";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also get a notification about which actual version was installed in the case where the version was not specified?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do this in a subsequent change

}
else
{
context.Publish(new DisplayedValueProduced($"Failed to add reference to package {package.PackageName}", context.Command));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When it fails, why does it fail, and is there additional information we can surface?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

message += $", version {package.PackageVersion}";
}

var key = message;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call this valueId so it's clearer what its significance is.

@rchande rchande merged commit 18b6fba into dotnet:master Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants