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

chore: refactor documentation generation #254

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

RomainMuller
Copy link
Contributor

Move the HTML template fragments to the generator package in order to improve the lisibility of the join points & advice codebases.

This allows using html/template templates instead of composing HTML into a buffer, then serializing it, and makes it easier to spot markup issues.

One caveat with this approach is that since this HTML gets embedded into a markdown document, care has to be taken to ensure that nothing gets indented more than 4 spaces deep if it is preceded by a blank line, as this is the syntax for Markdown indented code blocks (which cannot be disbaled).

Bonus features include:

  • Now rendering shorter names when referring to a single type, by showing only the package name instead of the entire import path (still available on hover, as it's the title attribute of an <abrr> tag)
  • Dropped a number of function options that were only used in unit tests

@RomainMuller RomainMuller requested a review from a team as a code owner August 30, 2024 13:56
@eliottness eliottness force-pushed the romain.marcadier/rm-dead-code branch 2 times, most recently from a195f8b to b7361ee Compare September 2, 2024 22:09
@eliottness eliottness marked this pull request as draft September 3, 2024 08:33
Base automatically changed from romain.marcadier/rm-dead-code to main September 16, 2024 14:17
Move the HTML template fragments to the `generator` package in order to
improve the lisibility of the join points & advice codebases.

This allows using `html/template` templates instead of composing HTML
into a buffer, then serializing it, and makes it easier to spot markup
issues.

One caveat with this approach is that since this HTML gets embedded into
a markdown document, care has to be taken to ensure that nothing gets
indented more than 4 spaces deep if it is preceded by a blank line, as
this is the syntax for Markdown indented code blocks (which cannot be
disbaled).

Bonus features include:
- Now rendering shorter names when referring to a single type, by
  showing only the package name instead of the entire import path (still
  available on hover, as it's the `title` attribute of an `<abrr>` tag)
- Dropped a number of function options that were only used in unit tests
@RomainMuller RomainMuller force-pushed the romain.marcadier/doc-html-templates branch from a691c02 to e6cc720 Compare September 30, 2024 11:34
@RomainMuller RomainMuller marked this pull request as ready for review September 30, 2024 11:35
Copy link
Contributor

@eliottness eliottness left a comment

Choose a reason for hiding this comment

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

I already reviewed most of it before you rebased it. LGTM 👍

@RomainMuller RomainMuller added this pull request to the merge queue Oct 1, 2024
Merged via the queue into main with commit c098377 Oct 1, 2024
25 checks passed
@RomainMuller RomainMuller deleted the romain.marcadier/doc-html-templates branch October 1, 2024 15:31
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

Attention: Patch coverage is 83.40611% with 38 lines in your changes missing coverage. Please review.

Project coverage is 73.88%. Comparing base (6f519f1) to head (e6cc720).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
internal/injector/builtin/generator/doc.go 80.00% 10 Missing and 7 partials ⚠️
internal/injector/builtin/generator/main.go 40.00% 4 Missing and 2 partials ⚠️
internal/injector/aspect/advice/code/template.go 61.53% 5 Missing ⚠️
internal/injector/aspect/join/function.go 87.50% 1 Missing and 4 partials ⚠️
internal/injector/aspect/advice/call.go 94.44% 1 Missing ⚠️
internal/injector/aspect/advice/struct.go 88.88% 0 Missing and 1 partial ⚠️
internal/injector/aspect/join/declaration.go 66.66% 0 Missing and 1 partial ⚠️
internal/injector/aspect/join/join.go 85.71% 1 Missing ⚠️
internal/injector/aspect/join/struct.go 94.44% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
+ Coverage   73.77%   73.88%   +0.10%     
==========================================
  Files         145      145              
  Lines        7958     7769     -189     
==========================================
- Hits         5871     5740     -131     
+ Misses       1658     1596      -62     
- Partials      429      433       +4     
Components Coverage Δ
Generators 76.98% <75.78%> (+0.29%) ⬆️
Instruments 88.05% <ø> (ø)
Go Driver 72.81% <ø> (ø)
Toolexec Driver 70.88% <ø> (ø)
Aspects 71.86% <88.80%> (-0.09%) ⬇️
Injector 73.72% <83.40%> (+0.29%) ⬆️
Job Server 63.20% <ø> (ø)
Integration Test Suite 87.80% <ø> (ø)
Other 73.88% <83.40%> (+0.10%) ⬆️
Files with missing lines Coverage Δ
internal/injector/aspect/advice/assign.go 66.66% <100.00%> (-2.30%) ⬇️
internal/injector/aspect/advice/block.go 67.85% <100.00%> (-2.15%) ⬇️
internal/injector/aspect/advice/import.go 81.25% <ø> (-2.09%) ⬇️
internal/injector/aspect/advice/inject.go 73.17% <100.00%> (-4.42%) ⬇️
internal/injector/aspect/advice/wrap.go 73.52% <100.00%> (-1.48%) ⬇️
internal/injector/aspect/join/all-of.go 70.00% <ø> (-5.48%) ⬇️
internal/injector/aspect/join/configuration.go 85.71% <ø> (+0.29%) ⬆️
internal/injector/aspect/join/directive.go 90.47% <ø> (-0.44%) ⬇️
internal/injector/aspect/join/expression.go 85.71% <100.00%> (-0.78%) ⬇️
internal/injector/aspect/join/not.go 80.00% <100.00%> (ø)
... and 11 more

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