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

Ported ReportGeneratorHelper to FAKE 5 #1887

Merged
merged 5 commits into from
Apr 26, 2018

Conversation

magicmonty
Copy link
Contributor

Ported ReportGeneratorHelper to FAKE 5 as Fake.Testing.ReportGenerator

@matthid
Copy link
Member

matthid commented Apr 23, 2018

Wow you are on a streak right now :). Thanks!

Can you take a look at the documentation around the new modules as well? Stuff needs to move into the correct section in the template and todo-*.md files need to be moved to legacy-*.md and we might need a reworked version (I'm not sure if the modules have existing legacy documentation).
You can take a look at what I have done here: #1881

It might very well possible that there are no existing docs then nothing needs to be done ...

@magicmonty
Copy link
Contributor Author

I added the documentation for the report generator. I will add the docs for the other PR's as well (but in the other PR's ;)

@@ -140,6 +140,7 @@
<a href="@(prefix)apidocs/index.html#Fake.Testing">Testing</a>
<ul>
<li><a href="@(prefix)testing-sonarqube.html">SonarQube</a></li>
<li><a href="@(prefix)testing-reportgenerator.html">ReportGenerator</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

Feel free to just link the API-Reference-documentation. I wasn't after adding a markdown file for each module (inline /// documentation works as well).

I was more after looking for todo- docs in https://github.com/fsharp/FAKE/tree/master/help/markdown but it looks like none of your modules was documented previously.

Anyway thanks so much! :)

@matthid matthid changed the base branch from master to rc_8 April 24, 2018 17:59
| Error = 2

/// ReportGenerator parameters, for more details see: https://github.com/danielpalme/ReportGenerator.
[<CLIMutable>]
Copy link
Member

Choose a reason for hiding this comment

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

remove

let private currentDirectory = Directory.GetCurrentDirectory ()

/// ReportGenerator default parameters
let ReportGeneratorDefaultParams =
Copy link
Member

Choose a reason for hiding this comment

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

private/internal?

let taskName = "ReportGenerator"
let description = "Generating reports"

Trace.traceStartTaskUnsafe taskName description
Copy link
Member

Choose a reason for hiding this comment

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

use non-unsafe version

@matthid
Copy link
Member

matthid commented Apr 24, 2018

I reviewed all PRs and they look very good, only some smaller things left. I changed the base branch to rc_8 as I'd like to add them to the next release :).

@magicmonty
Copy link
Contributor Author

@matthid Fixed

@matthid matthid merged commit 23b1bd9 into fsprojects:rc_8 Apr 26, 2018
@matthid
Copy link
Member

matthid commented Apr 26, 2018

Thanks!

@magicmonty magicmonty deleted the reportgenerator branch April 26, 2018 10:42
@matthid
Copy link
Member

matthid commented Apr 26, 2018

Good job

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.

2 participants