Skip to content

ci(docs): add new column detailing default provider enablement#57

Merged
mergify[bot] merged 1 commit intoopendatahub-io:mainfrom
nathan-weinberg:moar-docs
Oct 2, 2025
Merged

ci(docs): add new column detailing default provider enablement#57
mergify[bot] merged 1 commit intoopendatahub-io:mainfrom
nathan-weinberg:moar-docs

Conversation

@nathan-weinberg
Copy link
Copy Markdown
Collaborator

@nathan-weinberg nathan-weinberg commented Oct 2, 2025

What does this PR do?

some providers are enabled by default in our distro whereas others require an environmental variable to be set when the container boots

this commit makes that distinction clearer by adding it to our auto-generated docs

Summary by CodeRabbit

  • Documentation

    • Expanded the API/provider table to include an “Enabled by default?” column with clear Yes/No indicators.
    • Clarifies the default enablement status for each provider while keeping all existing entries and sorting intact.
    • Presentation-only update; no changes to provider behavior or APIs.
  • Chores

    • Updated the documentation generation process to produce the new column consistently across builds.

some providers are enabled by default in our distro whereas
others require an environmental variable to be set when
the container boots

this commit makes that distinction clearer by adding it to
our auto-generated docs

Signed-off-by: Nathan Weinberg <nweinber@redhat.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 2, 2025

Walkthrough

The documentation table in distribution/README.md now includes an “Enabled by default?” column. scripts/gen_distro_docs.py was updated to compute and emit this column by detecting conditional provider patterns and marking them as not enabled by default; non-conditional providers are marked enabled. Sorting and other behaviors remain unchanged.

Changes

Cohort / File(s) Summary
Docs: distro table update
distribution/README.md
Expanded the API/provider matrix from two to three columns by adding “Enabled by default?” with Yes/No values; retained existing entries.
Script: docs generation logic
scripts/gen_distro_docs.py
Added derivation of enabled_by_default based on provider_id conditional pattern ${...:+...}; updated header, delimiter, data tuples, and row emission to include the third column; preserved existing sorting order.

Sequence Diagram(s)

sequenceDiagram
    actor Dev as Developer
    participant Script as gen_distro_docs.py
    participant Providers as Provider Configs
    participant Table as Distro Table (README)

    Dev->>Script: Run docs generation
    Script->>Providers: Iterate API/provider pairs
    loop For each pair
        Script->>Script: Detect conditional pattern in provider_id<br/>(matches ${...:+...})
        alt Conditional pattern found
            Script->>Script: enabled_by_default = "No"
        else No conditional
            Script->>Script: enabled_by_default = "Yes"
        end
        Script->>Table: Append row (api_name, provider_type, enabled_by_default)
    end
    Script->>Table: Write header with 3 columns
    Note right of Table: Table now includes "Enabled by default?" column
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hopped through rows with tidy cheer,
A third small column now appears here.
“Yes” for fields that always show,
“No” where guarded flags may go.
Ears up high, I ship this doc—
Thump! Another neat, precise tick-tock. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title follows conventional commit style and succinctly captures the primary update, namely adding a new documentation column for default provider enablement. It directly aligns with the changes to the distribution README and the documentation generation script. This phrasing makes the purpose clear to any reviewer scanning the history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff047c3 and 3fc835a.

📒 Files selected for processing (2)
  • distribution/README.md (1 hunks)
  • scripts/gen_distro_docs.py (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build-test-push (linux/amd64)
  • GitHub Check: Summary
🔇 Additional comments (5)
distribution/README.md (1)

11-35: LGTM! Auto-generated table appears correctly formatted.

The new "Enabled by default?" column has been added consistently throughout the table. Since this file is auto-generated by scripts/gen_distro_docs.py, the substantive review should focus on the generator script logic.

scripts/gen_distro_docs.py (4)

41-44: LGTM! Table header correctly updated.

The new column header and delimiter are properly formatted and aligned with the existing columns.


69-69: LGTM! Sort logic correctly preserved.

The sort key lambda x: (x[0], x[1]) continues to sort by API name then provider type, appropriately ignoring the new enabled_by_default field. This maintains the existing sort behavior while accommodating the expanded tuple structure.


72-73: LGTM! Table row generation correctly updated.

The loop now unpacks three values and the f-string properly includes the new enabled_by_default field, maintaining alignment with the updated table header.


55-66: Conditional detection logic is correct and covers all current provider_id values.

No edge cases found in distribution/run.yaml; no changes needed.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.


# Check if provider_id contains the conditional syntax ${<something>:+<something>}
# This regex matches the pattern ${...} containing :+
is_conditional = bool(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if provider_id is missing from config, it looks it will default to empty string and gets treated as "enabled by default", right? do we want to handle that case or assume provider_id will be there always...

otherwise, LGTM @nathan-weinberg . Thanks!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Provider ID should always be there - otherwise it would be an invalid run.yaml and the CI would catch it elsewhere 😄

Copy link
Copy Markdown
Collaborator

@rhdedgar rhdedgar left a comment

Choose a reason for hiding this comment

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

Looks good to me! +1

@mergify mergify bot merged commit e33ccd5 into opendatahub-io:main Oct 2, 2025
6 checks passed
@nathan-weinberg nathan-weinberg deleted the moar-docs branch October 2, 2025 18:37
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.

3 participants