-
Notifications
You must be signed in to change notification settings - Fork 75
Change tool name from dotnet-fsharplint to fsharplint #778
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
base: master
Are you sure you want to change the base?
Conversation
18ecf92 to
b4acb3e
Compare
|
@Numpsy can you review? |
| <PackageType>DotNetCliTool</PackageType> | ||
| <PackAsTool>true</PackAsTool> | ||
| <AssemblyName>dotnet-fsharplint</AssemblyName> | ||
| <AssemblyName>fsharplint</AssemblyName> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@webwarrior-ws maybe we should rather use ToolCommandName here, as instructed by #746 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment that you've linked says that
It would have no impact on
dnx
b4acb3e to
63419cd
Compare
|
I sent @duckmatt (previous/original maintainer) an email yesterday about the permissions to publish new versions in the old nuget package: https://www.nuget.org/packages/FSharpLint , but maybe he disagrees? Or any of you might disagree too, about breaking possible users of the old library that might upgrade and spend some time figuring out that they have to migrate to "FSharpLint.Core"? If that's the case, another solution (which is also much shorter) would be to reserve the name "fslint" which seems available and would result in a much more neat UX due to the super short command (thanks to dnx too): |
|
I think Fantomas did a change like that some time ago ('fantomas' was the library and 'fantomas-tool' was the tool, and now 'fantomas' is the tool and 'fantomas.core' is the library). The download stats at https://www.nuget.org/stats/packages/FSharpLint?groupby=Version do show several hundred downloads of 'fsharplint' in the last 6 weeks despite it not having been updated since 2016, so that might upset something so maybe 'fslint' is cleaner. |
Of FSharpLint.Console from dotnet-fsharplint to fsharplint. This way it will be published as fsharplint package and the tool name will be just fsharplint, not dotnet-fsharplint.
From locally built package.
To reflect changes in tool name from dotnet-fsharplint to fsharplint.
|
Rebased to fix CI.
But can you review the docs changes, to make sure they really fix bug 746? Thanks @Numpsy |
|
I'll do a local build and try it (and dnx as well) |
|
|
||
| Run `dnx fsharplint --help` for full usage information. | ||
|
|
||
| # Installing and runnnig on .NET versions before 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'runnnig' should be 'running' ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
I tried building this branch locally and installing/running it with the 'fsharplint' name and that appeared to work following the documentation. I did however have an instance of #687 when I tried running
That worked after setting roll forward to latestMajor. That's not directly related to this change, but I'm not sure if it's something that might come up with dnx if that's just a .NET 10+ thing? |


Change
FSharpLint.Consoleproject's assembly name, fromdotnet-fsharplinttofsharplint. This way it will be published asfsharplintnuget package and, combined withdnxinvocation (which this PR also highlights in the docs), will get UX much leaner.Fixes #746