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

link to nugettrends.com #9737

Merged
merged 3 commits into from
Apr 15, 2024
Merged

Conversation

bruno-garcia
Copy link
Contributor

NuGet Trends is a .NET Foundation project: https://github.com/dotnet/nuget-trends

Adding a link on the package details' right panel.

Disclaimer: I'm on a Mac and can't really build things without several hacks locally so expect a few CI runs to get this right

@bruno-garcia bruno-garcia requested a review from a team as a code owner November 26, 2023 00:27
@JonDouglas
Copy link
Contributor

JonDouglas commented Nov 28, 2023

Hey @bruno-garcia,

Thank you for the PR!

Do you think this might fit better in our statistics detail page given what NuGet Trends does? We only expose 6 weeks of data. Your project exposes much more. Do you think your project would complement that instead of where you propose it in this PR?

Maybe something like this or some other location on this page?

image

I have initial hesitations on just including it on the package details page when the context is more stats oriented. Maybe in the stats/downloads section? Not sure right now.

What is the default view for statistics? 2 years? We can add specifics for that and figure out a good spot for it in my opinion. The larger team can likely comment further here too.

@bruno-garcia
Copy link
Contributor Author

Thanks for your input @JonDouglas:

Do you think this might fit better in our statistics detail page given what NuGet Trends does?
Do you think your project would complement that instead of where you propose it in this PR?

I agree it might fit well in that page, but discoverability is rather low compared with stacking up next to fuget and nuget package explorer. I use both often and I'd love to go straight to nuget trends from the package detail page if that was possible.

What is the default view for statistics? 2 years? We can add specifics for that and figure out a good spot for it in my opinion. The larger team can likely comment further here too.

Right, but we can specify that via query string. Default 2 years: (results in a redirect adding &months=24):
https://nugettrends.com/packages?ids=NuGet.CommandLine
vs specifying 10 years directly:
https://nugettrends.com/packages?months=120&ids=NuGet.CommandLine

The larger team can likely comment further here too.

Got it, looking forward to more input.

@bruno-garcia
Copy link
Contributor Author

Anyone else has input here? I'm still interested in moving forward with adding a way to jump into NuGet Trends from a package details page

@JonDouglas
Copy link
Contributor

@bruno-garcia Can you post a screenshot and edit your PR so people who view this PR can see what is proposed? Additionally, I can evangelize this PR during the holidays so we can get more community perspectives.

I do not think we will have a collective decision until the new year however. Many are beginning their vacations here in the USA.

@mariaghiondea
Copy link
Contributor

Thank you, @bruno-garcia! It's awesome to see community contributions and the benefit to everyone.
I like Jon's proposal of exploring placing options and getting the community's input on the best one. We'll keep you updated.

@bruno-garcia
Copy link
Contributor Author

Thanks @mariaghiondea and @JonDouglas for the input.
I'm struggling to get this running on a Mac and I don't have a Windows machine.
I'll see if I get a Windows PC I can run this branch and take a screenshot.

Any chance we'll have previews release of branches (akin Vercel) as a Christmas gift? 😅

@JonDouglas
Copy link
Contributor

@bruno-garcia after consideration and getting back to this, we're cool to just include it as is and see what feedback we get. Since it is behind a feature flag today, if people do ask about it or want it removed we can track that feedback and make a future decision. But for now, let's just move forward as is! Thanks again for this contribution.

For next steps, someone from the @NuGet/gallery-team will help in getting this merged and staging it & eventually pushed to prod over the next few weeks.

mariaghiondea
mariaghiondea previously approved these changes Feb 2, 2024
JonDouglas
JonDouglas previously approved these changes Feb 2, 2024
@bruno-garcia
Copy link
Contributor Author

That's great! Thanks a lot folks!

@bruno-garcia
Copy link
Contributor Author

Looking forward to next steps!

@mariaghiondea
Copy link
Contributor

I forgot to post an update, so thank you for the reminder @bruno-garcia.
We are working on expanding our CI build pipeline capabilities to build community PRs. Once that happens, this will be the first PR to go through the new process. Thank you for your patience.

@mariaghiondea
Copy link
Contributor

Update: we are working on updating our pipelines for compliance reasons and this is on hold until that is done (to not waste efforts). We are trying to complete by end of this month.

@lyndaidaii lyndaidaii changed the base branch from main to dev March 25, 2024 23:51
@lyndaidaii lyndaidaii dismissed stale reviews from JonDouglas and mariaghiondea March 25, 2024 23:51

The base branch was changed.

@lyndaidaii
Copy link
Contributor

This is how UI looks like on package detail page: it looks good to me. @JonDouglas @mariaghiondea, what do you think?

image

@lyndaidaii
Copy link
Contributor

@bruno-garcia, could you update PR to include those changes? thanks. PR: bruno-garcia#1

@erdembayar
Copy link
Contributor

We are closing this issue since we didn't hear back from you, but you can re-open or create new PR if necessary.
Happy coding!

@erdembayar erdembayar closed this Apr 9, 2024
add interface in fake feature service
@bruno-garcia
Copy link
Contributor Author

Oh, sorry I didn't check my notifications for a while. Merged the PR @lyndaidaii thanks!

@bruno-garcia
Copy link
Contributor Author

Doesn't seem like I can reopen :~

@erdembayar erdembayar reopened this Apr 10, 2024
@erdembayar
Copy link
Contributor

@bruno-garcia, could you update PR to include those changes? thanks. PR: bruno-garcia#1

@bruno-garcia I reopened the PR for you. Did you make the changes which was asked above?

@bruno-garcia
Copy link
Contributor Author

@bruno-garcia, could you update PR to include those changes? thanks. PR: bruno-garcia#1

@bruno-garcia I reopened the PR for you. Did you make the changes which was asked above?

yeah, got merged in! thanks folks. Let me know what's next

@bruno-garcia
Copy link
Contributor Author

NuGetGallery - CI Expected — Waiting for status to be reported

Did the job with that name get removed or renamed? Looks like a branch protection was set but the job is unavailable?

@joelverhagen
Copy link
Member

NuGetGallery - CI Expected — Waiting for status to be reported

Apologies for the delay. We need to manually approve each commit to community PRs for our CI. I've done this now.

NuGetGallery - CI Pending — in progress

@bruno-garcia
Copy link
Contributor Author

woo thanks folks!

looks all green 🚀 can't wait to see it live

@lyndaidaii lyndaidaii merged commit b6e8d97 into NuGet:dev Apr 15, 2024
2 checks passed
@lyndaidaii
Copy link
Contributor

@bruno-garcia, thank you for your contribution. Will let you know once it's live on NuGet.org. thank you so much!

@bruno-garcia bruno-garcia deleted the feat/nuget-trends branch April 17, 2024 14:57
Goodyear2017 pushed a commit to Goodyear2017/NuGetGallery that referenced this pull request Apr 22, 2024
* link to nugettrends.com

* add interface in fake feature service

Co-authored-by: Lynn Dai
@joelverhagen
Copy link
Member

joelverhagen commented May 10, 2024

Hey @bruno-garcia, we detected a bug in this PR where the image does not show up properly
image
DEV environment link: https://dev.nugettest.org/Content/gallery/img/nuget-trends.svg, https://dev.nugettest.org/Content/gallery/img/nuget-trends-32x32.png

I believe it is because the PNG and SVG were not added a content files, similar to the similar links (See FuGet below):

<Content Include="Content\gallery\img\fuget-32x32.png" />
<Content Include="Content\gallery\img\fuget.svg" />

Would you be interested in making a quick PR to main (hotfix path) to fix this?

It looks like we've missed this before (#8385) 😅

@bruno-garcia
Copy link
Contributor Author

Oh sorry didn't see this before, and I'm going away for a week tomorrow 6 AM.
I can look at this in a week though.

bruno-garcia added a commit to bruno-garcia/NuGetGallery that referenced this pull request May 29, 2024
@bruno-garcia
Copy link
Contributor Author

Hey @bruno-garcia, we detected a bug in this PR where the image does not show up properly image DEV environment link: dev.nugettest.org/Content/gallery/img/nuget-trends.svg, dev.nugettest.org/Content/gallery/img/nuget-trends-32x32.png

I believe it is because the PNG and SVG were not added a content files, similar to the similar links (See FuGet below):

<Content Include="Content\gallery\img\fuget-32x32.png" />
<Content Include="Content\gallery\img\fuget.svg" />

Would you be interested in making a quick PR to main (hotfix path) to fix this?

It looks like we've missed this before (#8385) 😅

Finally back, PR: #9982

joelverhagen pushed a commit that referenced this pull request May 30, 2024
@joelverhagen
Copy link
Member

@bruno-garcia, we've enabled this on PROD. Thanks again for your contribution and your stewardship of this awesome community tool!
image

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.

6 participants