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

Package as a DotNetCliTool #130

Merged
merged 3 commits into from May 9, 2018

Conversation

hunterjm
Copy link
Contributor

@hunterjm hunterjm commented May 2, 2018

This will enable someone to add a <DotNetCliToolReference> reference to their test project and call the assembly using something similar to:

dotnet ReportGenerator -reports:coverage.xml -reporttypes:HTML -targetdir:report

I've tested this using both the dotnet cli on a Mac and the ReportGenerator.exe assembly on a Windows machine.

@hunterjm
Copy link
Contributor Author

hunterjm commented May 3, 2018

Just noticed that I didn't test all the report formats, and some of those assemblies are required. Will be updating this PR shortly.

…erator.Core, setup ReportGenerator.Core to be released as independant nuget package
@danielpalme
Copy link
Owner

Thanks for your PR. I have some questions on the changes you made:

  • Is there a technical reason why you moved the ReportGenerator.Reporting into ReportGenerator.Core?
  • You created a separate Nuget package for Core. Couldn't we just put everything into a single package? One package that supports all platforms?

@hunterjm
Copy link
Contributor Author

hunterjm commented May 3, 2018

@danielpalme - I moved things from ReportGenerator.Reporting to ReportGenerator.Core because there were some cyclical references going on Where ReportGenerator.Core was referencing ReportGenerator.Reporting even though there was no PackageReference (and couldn't be because ReportGenerator.Reporting is referencing ReportGenerator.Core). That's usually a sign that they should be the same project.

As for the separate Nuget package for Core, I created that mostly out of the desire to implement ReportGenerator's output into https://github.com/tonerdo/coverlet without having to run a separate CLI command. Having the netstandard2.0 lib available as a separate package will let coverage tools reference the Generator functionality directly.

In my companies nuget feed, I currently have custom ReportGenerator.Core and coverlet.msbuild packages that work together by issuing dotnet test /p:CollectCoverage=true /p:CoverletReportTypes=HTML which executes the tests, coverage collection, and report generation all in a single command.

With this PR, there are 3 ways to generate reports.

  1. Adding a DotNetCliToolReference to the test project of a netcoreapp2.0 project and letting nuget resolve dependancies. Execute using dotnet ReportGenerator {options}.
  2. Running ReportGenerator.exe directly from the tools path of the installed package on your machine
  3. Importing ReportGenerator.Core directly in your coverage library as a PackageReference and calling new Generator().GenerateReport() on the output file you have just created when collecting coverage.

@hunterjm
Copy link
Contributor Author

hunterjm commented May 3, 2018

As a clarification, the second NuGet package is the netstandard library, not the console app. Both the dotnet and 4.7 console apps are still packaged in the same package supporting both platforms.

@danielpalme
Copy link
Owner

I'm in the process of merging your PR. I spent some time to figure out which DLLs are really required in the Nuget package.

Again some questions:

  • Is there a technical reason to rename the output of "ReportGenerator.Console.NetCore.csproj" from ReportGeneator.dll to dotnet-ReportGenerator?
  • Do you have a link to some documention how to build a DotNetCliTool package?
  • Have you tested installing the generated Nuget package into another project? I tried with a local repository and ran into this issue: VS 2017: PackageType=DotnetCliTool fails to install NuGet/Home#4190

@hunterjm
Copy link
Contributor Author

hunterjm commented May 7, 2018

The first and second bullet can be explained with the Tools Extensibility Documentation. tl;dr; - currently the dotnet cli requires assembly files to be prefixed by "dotnet-" in order to run with "dotnet MyName"

For the third bullet point, I only added it to a new netcoreapp2.0 project by directly adding it to the .csproj. I'm not sure if that breaks backwards compatibility with the ".exe" assembly. Do people usually add the package to their project in order to use the EXE tool?

If not, I'd be happy to modify README to make it clear what needs to be done to install as a tool for use with "dotnet ReportGenerator [options]".

@hunterjm
Copy link
Contributor Author

hunterjm commented May 8, 2018

@danielpalme - I did a little more research, and this documentation suggests that we might actually not be able to package both together because of the DotNetCliTool package type. We could do what XUnit does, and create a separate "dotnet-reportgenerator" package like they do with "dotnet-xunit".

That would give us a total of 3 packages:

  • dotnet-reportgenerator
  • ReportGenerator
  • ReportGenerator.Core

Let me know your thoughts and I'd be happy to update this PR to reflect that structure. I've got the changes locally, and can add the ReportGenerator package to a .NET Framework console app and access ReportGenerator.exe through the PM console as well as add the DotNetCliToolReference and generate reports on a .NET Core app with "dotnet reportgenerator".

@danielpalme
Copy link
Owner

Thanks for your input, i will split the Nuget packages myself and add some help/description to guide users to the "correct" package.
I will also have a look how I can support the upcoming .NET Core Global Tools

Do people usually add the package to their project in order to use the EXE tool?
I think so,

@danielpalme
Copy link
Owner

danielpalme commented May 8, 2018

Current status:

  • Split packages
  • Custom icons for different packages
  • Add .NET Core Global Tools support
  • Local test of packages
  • Update README and package descriptions to guide users

@danielpalme danielpalme merged commit b31ef2e into danielpalme:v4 May 9, 2018
@danielpalme danielpalme added this to In progress in Release v4 May 10, 2018
@danielpalme danielpalme moved this from In progress to Done in Release v4 May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Release v4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants