Alternate install command for global tools packages on the packages page #5684

nkolev92 · 2018-03-26T07:08:35Z

Addresses #5182 .

Packages with this package type should display a “dotnet install packageId packageVersion” tab only, rather than the standard tabs.

The options I've considered are:

Display the correct message for dotnet.exe. Display unsupported for others.
This is what I've implemented here.

Display the correct message for dotnet.exe. Don't display tabs for others.

We need a review from PMs before merging this.
@anangaur @karann-msft
I'll sync up offline as well.

@skofman1 Please let me know if I should target a different branch in order to get this through live soon.

//cc for visibility.
@KathleenDollard
@wli3
@rrelyea

wli3 · 2018-03-26T17:22:37Z

it should be

dotnet tool install ---global PackageID

wli3 · 2018-03-26T17:23:58Z

src/NuGetGallery/Views/Packages/DisplayPackage.cshtml

-            InstallPackageCommand = string.Format("dotnet add package {0} --version {1}", Model.Id, Model.Version)
+            InstallPackageCommand = !packageIsTool ?
+            string.Format("dotnet add package {0} --version {1}", Model.Id, Model.Version) :
+            string.Format("dotnet tool install {0} --version {1}", Model.Id, Model.Version)


dotnet tool install --global {0} --version {1}

joelverhagen · 2018-03-26T17:52:59Z

I don't think we should display "unsupported". I think we should just hide the tab. But yeah, we need to talk to @anangaur.

joelverhagen · 2018-03-26T17:53:32Z

Also, how is this effected by #5656?

/cc @khellang

khellang · 2018-03-26T17:58:56Z

This is pretty nice! I don't think it affects #5656 much. I'd just have to rebase on top of this and check packageIsTool.

If it's a tool, I can output a DotNetCliToolReference element instead of a PackageReference. I think that means we should ditch the "PackageReference" name for the tab once and for all and go for "Project File" instead.

anangaur · 2018-03-26T18:30:50Z

Showing unsupported is not right. We should hide the other tabs. And show only the .NET CLI tab:

We should get the warning text (may be convert from warning to info text with gray background) from @KathleenDollard . Learn more in the text should take to global tools intro documentation. Again get a permalink from @KathleenDollard

nkolev92 · 2018-03-26T18:51:24Z

@khellang
This is not about dotnetCliToolReferences.
Those are being deprecated.
This about the new dotnet tool type- global tools, which cannot be installed in projects, and it only works via the CLI.

@wli3
Thanks, will correct it.

@anangaur
Thanks, I will add that as well.

Waiting on @KathleenDollard for the link.

khellang · 2018-03-26T18:52:26Z

This is not about dotnetCliToolReferences.
Those are being deprecated.

Oh. That didn't last long. Does that mean there's no way to install project-local tools (from the project file) anymore? I though the global tools were an addition, not a replacement?

nkolev92 · 2018-03-26T18:59:18Z

Does that mean there's no way to install project-local tools anymore? I though the global tools were an addition, not a replacement?

@khellang
We can't give you definitive answers to those questions. The CLI team has better context on that.

Short term, tools are not project local.
They're working on enabling more scenarios with these tools (not just user-wide tools)

wli3 · 2018-03-26T21:28:04Z

Does that mean there's no way to install project-local tools anymore? I though the global tools were an addition, not a replacement?

The packages of dotnetCliToolReferences and global tools are not compatible. global tools will replace dotnetCliToolReferences, but so far there is change on dotnetCliToolReferences. It should keep working

anangaur · 2018-03-27T05:43:51Z

@nkolev92 do attach the screenshot post implementation

nkolev92 · 2018-03-27T05:48:32Z

@anangaur
Will do :)
I was going to sync up offline with you on how exactly it should look before I added a screenshot here :D

nkolev92 · 2018-03-27T06:25:19Z

@anangaur
Updated with another screenshot.
The link is not added yet.
I'll wait for the appropriate link from .NET team

anangaur · 2018-03-27T14:54:15Z

Only the link to .NET tool is missing. Otherwise LGTM.

anangaur

Pending link.

wli3 · 2018-03-27T17:30:13Z

Kathleen is on a conference this week. Could this link be added later?

khellang · 2018-03-27T17:35:14Z

I can add it when I'm finishing #5656 👼

joelverhagen · 2018-03-27T18:44:50Z

src/NuGetGallery/ViewModels/PackageViewModel.cs

@@ -71,6 +72,7 @@ public PackageViewModel(Package package)
        public bool Prerelease { get; set; }
        public int DownloadCount { get; set; }
        public bool Listed { get; set; }
+        public bool IsDotnetToolPackageType { get; set; }


No particular reason.

I was following the conventions, because all of the above fields had setters as well, despite only being initialized in the constructor.

I can remove it though

wli3 · 2018-03-29T22:12:39Z

@nkolev92 wait for Kathleen? She will be back next week

joelverhagen

Looks great. Let me know when you've finalized the link URL and text and we can work on validating the change in our DEV environment.

khellang · 2018-04-02T19:40:06Z

src/NuGetGallery/Views/Packages/DisplayPackage.cshtml

+                InstallPackageCommand = string.Format("dotnet add package {0} --version {1}", Model.Id, Model.Version)
+            },
+
+            new ThirdPartyPackageManagerViewModel("https://fsprojects.github.io/Paket/contact.html")


Why even keep ThirdPartyPackageManagerViewModel around at this point? The only property it adds is ContactUrl, but that property isn't used anymore (the contact url is just part of the message).

@khellang

The third party manager message is consistent, having it in the model will remove the need for duplication of that string.
It's a fraction cleaner to have it this way :)

KathleenDollard · 2018-04-03T22:10:17Z

Sorry I held this up.

If we're going with the last choice in the screen shots, is there still a link needed?

Is the info message the same for all tools or specific to the tool?

Just wanting to be clear on what text/links you need from me so we can get this tied up.

anangaur · 2018-04-03T22:19:24Z

@KathleenDollard A link is needed for users not aware of what tools are so that they can land on the doc page for global tools.

anangaur · 2018-04-03T22:21:11Z

The message will be something like:

This is a package that lets you install a tool in the .NET environment. Learn more

@KathleenDollard, can you also review the text of the message if this seems fine?

KathleenDollard · 2018-04-03T22:27:30Z

What confuses me is that the package is a tool, it doesn't let you install a tool. So, would this make sense (trying to make sure we are on the same page)

Installing this package will allow you to call it from a shell/command line. Learn more

Where Learn more is that we put it on your path and the security implications? Is that what you have in mind?

@peterhuene

Adding @mairaw for ideas on best place to link to.

mairaw · 2018-04-03T22:53:58Z

We don't have any public docs ready for global tools yet. When is this needed?

nkolev92 · 2018-04-03T23:17:31Z

Installing this package will allow you to call it from a shell/command line. Learn more

Is it clear enough what's meant by it ?
Because package names and tool names don't have to be congruent.

anangaur · 2018-04-03T23:27:10Z

May be:

This package can only be installed as a .NET tool in the development environment. Learn more

mairaw · 2018-04-03T23:51:15Z

Usually, our links to learn more look like: For more information, see topic name.

This helps with SEO instead of having generic link texts like here or learn more.

wli3 · 2018-04-05T16:32:20Z

Do we have an agreement on the link? @anangaur @mairaw @KathleenDollard If not could we have a meeting to decide it? also @nkolev92 , there is no technical issue blocking this PR right?

nkolev92 · 2018-04-05T16:47:27Z

@wli3
Nope. Just waiting on a consensus message and link.

mairaw · 2018-04-05T16:54:09Z

@KathleenDollard do we have anything in the form of spec or design notes for global tools somewhere? I could create a fwlink that would point to a temporary location for this until the official docs are not published.
I'm super busy this week working on a different release so I can't drop everything to go work on this instead.

nkolev92 · 2018-04-05T17:32:00Z

Having a fwlink is probably a great idea regardless.
We shouldn't need a change on the server if a docs reorg happens.

KathleenDollard · 2018-04-05T23:04:25Z

@mairaw Since this link will be to the docs, can you create a fwlink that makes sense. I will be working on content for this.

Maira and I met on this. This link will be to using tools, what they are any what the user should consider. That article will probably have a link to creating them.

KathleenDollard · 2018-04-09T21:26:22Z

I suggest:

This package contains a .NET Core Global Tool you can call from the shell/command line.

I think this should clearly state that this isn't a Visual Studio tool, and supply the link in a consistent manner.

@miraw, can you set up the fwlink or aka link?

nkolev92 · 2018-04-09T22:46:03Z

I think that message is great.
I'll update the PR as soon as I have the fwlink.

KathleenDollard · 2018-04-10T20:18:38Z

fwlink: https://aka.ms/global-tools

nkolev92 · 2018-04-11T16:45:35Z

@joelverhagen @skofman1
Updated the messaging and link.

Happy to test this out on dev whenever.

skofman1 · 2018-04-11T17:43:47Z

@nkolev92 , looks good! feel free to merge!

nkolev92 requested review from joelverhagen, agr, loic-sharma, ryuyu, anangaur, chenriksson, cristinamanum, skofman1 and scottbommarito March 26, 2018 07:08

nkolev92 changed the base branch from master to dev March 26, 2018 07:08

wli3 reviewed Mar 26, 2018

View reviewed changes

alternate command for tools packages

72e0e1b

nkolev92 force-pushed the dev-nkolev92-5182 branch from 0f438ae to 72e0e1b Compare March 27, 2018 06:43

anangaur approved these changes Mar 27, 2018

View reviewed changes

joelverhagen reviewed Mar 27, 2018

View reviewed changes

nkolev92 added 2 commits March 29, 2018 15:22

indentataion

0596327

indentation

33821e4

joelverhagen approved these changes Apr 2, 2018

View reviewed changes

khellang reviewed Apr 2, 2018

View reviewed changes

spboyer mentioned this pull request Apr 4, 2018

.NET 2.1 - Add flag or indicator for global tool searching / discoverability #5762

Closed

change message

1172511

nkolev92 force-pushed the dev-nkolev92-5182 branch from 7663b85 to 1172511 Compare April 11, 2018 16:43

Merge branch 'dev' into dev-nkolev92-5182

ba21841

nkolev92 merged commit 3c5a43c into dev Apr 11, 2018

nkolev92 deleted the dev-nkolev92-5182 branch April 11, 2018 19:47

khellang mentioned this pull request Apr 8, 2019

Add tab for PackageReference #5656

Merged

Alternate install command for global tools packages on the packages page #5684

Alternate install command for global tools packages on the packages page #5684

Conversation

nkolev92 commented Mar 26, 2018 • edited Loading

wli3 commented Mar 26, 2018

wli3 Mar 26, 2018 • edited Loading

Choose a reason for hiding this comment

nkolev92 Mar 27, 2018

Choose a reason for hiding this comment

joelverhagen commented Mar 26, 2018

joelverhagen commented Mar 26, 2018

khellang commented Mar 26, 2018 • edited Loading

anangaur commented Mar 26, 2018

nkolev92 commented Mar 26, 2018

khellang commented Mar 26, 2018 • edited Loading

nkolev92 commented Mar 26, 2018

wli3 commented Mar 26, 2018 • edited Loading

anangaur commented Mar 27, 2018

nkolev92 commented Mar 27, 2018

nkolev92 commented Mar 27, 2018 • edited Loading

anangaur commented Mar 27, 2018

anangaur left a comment

Choose a reason for hiding this comment

wli3 commented Mar 27, 2018

khellang commented Mar 27, 2018

joelverhagen Mar 27, 2018

Choose a reason for hiding this comment

nkolev92 Mar 27, 2018

Choose a reason for hiding this comment

wli3 commented Mar 29, 2018

joelverhagen left a comment

Choose a reason for hiding this comment

khellang Apr 2, 2018

Choose a reason for hiding this comment

nkolev92 Apr 2, 2018 • edited Loading

Choose a reason for hiding this comment

KathleenDollard commented Apr 3, 2018

anangaur commented Apr 3, 2018

anangaur commented Apr 3, 2018

KathleenDollard commented Apr 3, 2018

mairaw commented Apr 3, 2018

nkolev92 commented Apr 3, 2018

anangaur commented Apr 3, 2018

mairaw commented Apr 3, 2018

wli3 commented Apr 5, 2018

nkolev92 commented Apr 5, 2018

mairaw commented Apr 5, 2018

nkolev92 commented Apr 5, 2018

KathleenDollard commented Apr 5, 2018

KathleenDollard commented Apr 9, 2018

nkolev92 commented Apr 9, 2018

KathleenDollard commented Apr 10, 2018

nkolev92 commented Apr 11, 2018

skofman1 commented Apr 11, 2018

nkolev92 commented Mar 26, 2018 •

edited

Loading

wli3 Mar 26, 2018 •

edited

Loading

khellang commented Mar 26, 2018 •

edited

Loading

khellang commented Mar 26, 2018 •

edited

Loading

wli3 commented Mar 26, 2018 •

edited

Loading

nkolev92 commented Mar 27, 2018 •

edited

Loading

nkolev92 Apr 2, 2018 •

edited

Loading