Skip to content
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

Add fuget.org links to packages #8364

Merged
merged 2 commits into from
Jan 7, 2021
Merged

Add fuget.org links to packages #8364

merged 2 commits into from
Jan 7, 2021

Conversation

drewgillies
Copy link
Contributor

@drewgillies drewgillies commented Dec 23, 2020

Addresses: #7850

Packages will now have an additional link per specs outlined here: #7832, linking to the package's fuget.org view.

image

@drewgillies
Copy link
Contributor Author

drewgillies commented Dec 23, 2020

FYI @chgill-MSFT @anangaur
Chris--I've used a fallback png for the icon which was simply scraped from the other PR (I've used the provided SVG as the main icon though). If you want to include a better fallback, let me know.

@drewgillies drewgillies mentioned this pull request Dec 23, 2020
Copy link
Contributor

@skofman1 skofman1 left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -163,6 +163,12 @@ public DisplayPackageViewModelFactory(IIconUrlProvider iconUrlProvider)
viewModel.ProjectUrl = projectUrl;
}

var fugetUrl = $"https://www.fuget.org/packages/{package.Id}/{package.Version}";
Copy link
Member

Choose a reason for hiding this comment

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

is this the "full version" or the "normalized version"? We should use the normalized version because that includes all URL safe characters. The build metadata is included in the full and original, non-normalized version strings. So 1.0.0+foo is a valid version but we only want 1.0.0 in the URL. To be safe, it's best to use URL encoding methods to build this URL so you don't have to think about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to NormalizedVersion. Thanks!

[Theory]
[InlineData("foo", "1.0.0", "https://www.fuget.org/packages/foo/1.0.0")]
[InlineData("foo", "1.1.0", "https://www.fuget.org/packages/foo/1.1.0")]
[InlineData("foo.bar", "1.1.0-beta", "https://www.fuget.org/packages/foo.bar/1.1.0-beta")]
Copy link
Member

Choose a reason for hiding this comment

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

nit: add uppercase package IDs and versions as well as versions with build metadata to assert the respective behaviors

@drewgillies drewgillies merged commit 86af6b6 into dev Jan 7, 2021
@drewgillies drewgillies deleted the dg-add-fuget-links branch January 7, 2021 23:50
@zhhyu zhhyu mentioned this pull request Jan 28, 2021
17 tasks
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.

4 participants