Create pipeline stage to generate list of supported resources #1791
Conversation
theunrepentantgeek
requested review from
babbageclunk,
davefellows,
matthchr and
Porges
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1791 +/- ##
==========================================
+ Coverage 42.01% 42.03% +0.01%
==========================================
Files 319 320 +1
Lines 90719 90773 +54
==========================================
+ Hits 38117 38157 +40
- Misses 49244 49257 +13
- Partials 3358 3359 +1
Continue to review full report at Codecov.
|
|
This is related to #1715 If we had #1715 would we want to keep this? In some ways it gives a nice summary of the resources in each API folder, which is neat. In other ways, it would be covered by #1715 from a "documentation for customers" perspective, because at that point we'd be pointing them at the generated docs and not here. If you think this should be removed once we have #1715, possibly put a comment someplace indicating that. |
| // ReportResourceVersionsStageID is the unique identifier of this stage | ||
| const ReportResourceVersionsStageID = "reportResourceVersions" | ||
|
|
||
| // ReportResourceVersions creates a pipeline stage that removes any wrapper types prior to actual code generation |
There was a problem hiding this comment.
This comment doesn't seem right?
|
|
||
| ### microsoft.network | ||
|
|
||
| v1alpha1api20201101 |
There was a problem hiding this comment.
Consider bolding this for emphasis?
There was a problem hiding this comment.
It's the only thing on the line, and it's followed by a bullet list, so I don't think it needs further emphasis.
| } | ||
|
|
||
| // Summarize collates a list of all resources, grouped by package | ||
| func (r *ResourceVersionsReport) Summarize(types astmodel.Types) { |
There was a problem hiding this comment.
This is a style thing, but it feels sorta weird to me to first construct an empty one of these, and then call Summarize on it causing its immediate mutation. You could do all of this in the NewResourceVersionsReport call and then your type is immutable, which feels like a win. This fits especially nicely here because this iteration can't error anyway (which would be a reason to not do it in the New).
There was a problem hiding this comment.
Good idea. Changed.
| } | ||
|
|
||
| func (r *ResourceVersionsReport) WriteTo(outputPath string) error { | ||
|
|
There was a problem hiding this comment.
very minor: You have a newline here but not on the other methods
|
I don't think this would necessarily be supplanted by #1715; it would depend on what information is included in the output. |
What this PR does / why we need it:
Generates a report that lists all the resources (and versions) included in the generation run.
How does this PR make you feel:
If applicable: