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

Render documentation for provider init callbacks #224

Merged
merged 10 commits into from
May 22, 2024

Conversation

tetromino
Copy link
Collaborator

If a provider has a custom init callback, we want the summary blurb to show init's parameters (since these are what the user will interact with); we have to render constructor paramaters and fields separately (and using separate html anchors) since there is not necessarily a 1-1 relationship between them, and since they may have different docs.

Fixes #182

If a provider has a custom `init` callback, we want the summary blurb
to show `init`'s parameters (since these are what the user will interact
with); we have to render constructor paramaters and fields separately
(and using separate html anchors) since there is not necessarily a 1-1
relationship between them, and since they may have different docs.

Fixes bazelbuild#182
@tetromino tetromino requested a review from brandjon as a code owner May 20, 2024 20:54
@tetromino tetromino requested a review from comius May 20, 2024 20:57
@tetromino tetromino marked this pull request as draft May 20, 2024 22:19
@tetromino
Copy link
Collaborator Author

tetromino commented May 20, 2024

Current PR is not ready. I need to fix the following problems which occurred to me in the process of testing it:

  • In virtually all cases, the init callback's summary docstring is not interesting to the user (it's along the lines of "Validates params foo and bar") - so we should not render it. The provider's docstring is the interesting part for the user.
    • done
  • Typically, the list of parameters of the init callback exactly matches the list of fields. If that is so, we should merge parameters and fields into a single table.
  • And even if the set of init parameters is different from fields, we should propagate docstrings from fields to undocumented init parameters with the same name.

In other words, fairly extensive munging of the ProviderInfo proto is necessary.

By default, we want the following behavior:

* Custom init callback specified
  * The set of parameters for the init callback equals the set of
    fields for the provider; and the docs for the init callback's
    parameters are either empty or equal to corresponding field docs
    * Some init parameters have a default value:
      -> Render a single "Fields" table with 3 columns (name, doc,
         default value)
    * ... otherwise
      -> Render a single "Fields" table with 2 columns
  * ... otherwise
    -> Render two tables - "Constructor parameters" and "Fields" - with
       the links from the summary blurb (interfixed with "_init")
       leading to the parameters table (not the fields table)
* ... otherwise
  -> Trivial case - single "Fields" table (as before).
@tetromino tetromino marked this pull request as ready for review May 21, 2024 21:35
@tetromino
Copy link
Collaborator Author

I think by default, we want the following behavior:

  • Custom init callback specified
    • The set of parameters for the init callback equals the set of
      fields for the provider; and the docs for the init callback's
      parameters are either empty or equal to corresponding field docs
      • Some init parameters have a default value:
        -> Render a single "Fields" table with 3 columns (name, doc,
        default value)
      • ... otherwise
        -> Render a single "Fields" table with 2 columns
    • ... otherwise
      -> Render two tables - "Constructor parameters" and "Fields" - with
      the links from the summary blurb (interfixed with "_init")
      leading to the parameters table (not the fields table)
  • ... otherwise
    -> Trivial case - single "Fields" table (as before).

Copy link
Member

@brandjon brandjon left a comment

Choose a reason for hiding this comment

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

Discussed offline, approved but mind the rendering failure in one of the goldens,

@tetromino tetromino merged commit 666b7ba into bazelbuild:master May 22, 2024
16 checks passed
@tetromino tetromino deleted the provider-constructor branch May 22, 2024 19:42
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.

Provider init callback needs to be documented
2 participants