Skip to content

Fix html_reports feature#86

Open
WaterWhisperer wants to merge 1 commit into
criterion-rs:masterfrom
WaterWhisperer:fix/html-reports-feature
Open

Fix html_reports feature#86
WaterWhisperer wants to merge 1 commit into
criterion-rs:masterfrom
WaterWhisperer:fix/html-reports-feature

Conversation

@WaterWhisperer

Copy link
Copy Markdown

Fixes #56

Copilot AI review requested due to automatic review settings May 23, 2026 06:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR restores html_reports as the switch that actually controls HTML report generation, making HTML-report-related deps truly optional and updating docs/tests to reflect the default-feature behavior.

Changes:

  • Gate HTML-report code paths behind feature = "html_reports" and make criterion-plot/tinytemplate optional deps enabled by that feature.
  • Add a regression test to ensure disabling html_reports prevents report/ output.
  • Update README/book docs and changelog to reflect the feature/defaults behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/criterion_tests.rs Adds a feature-gated regression test for absence of HTML report output.
src/report.rs Guards the HTML reporter field and invocation behind html_reports.
src/lib.rs Gates plotting modules/APIs behind html_reports and adds no-op APIs when disabled.
book/src/user_guide/plots_and_graphs.md Updates guidance/examples around enabling/disabling HTML reports.
book/src/user_guide/html_report.md Updates guidance/examples around enabling/disabling HTML reports.
README.md Simplifies dependency snippet now that HTML reports are default-enabled.
Cargo.toml Makes HTML-report deps optional and adds html_reports to default features.
CHANGELOG.md Notes fix restoring html_reports control over report generation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/criterion_tests.rs
Comment thread Cargo.toml Outdated
Comment thread Cargo.toml
Comment thread book/src/user_guide/html_report.md Outdated
Comment thread book/src/user_guide/html_report.md Outdated
@WaterWhisperer WaterWhisperer force-pushed the fix/html-reports-feature branch from 3d0ab95 to 4a65207 Compare May 23, 2026 07:13
@lemmih

lemmih commented May 25, 2026

Copy link
Copy Markdown
Contributor

Making 'html_reports' a default feature goes beyond just fixing it. How about we add tests for the feature while keeping it non-default?

@WaterWhisperer

Copy link
Copy Markdown
Author

Making 'html_reports' a default feature goes beyond just fixing it. How about we add tests for the feature while keeping it non-default?

Thanks for clarifying. I treated the issue suggestion’s “enable it by default” as part of the fix, I'll update it.

Copilot AI review requested due to automatic review settings May 25, 2026 14:10
@WaterWhisperer WaterWhisperer force-pushed the fix/html-reports-feature branch from 4a65207 to 8802593 Compare May 25, 2026 14:10
@WaterWhisperer WaterWhisperer force-pushed the fix/html-reports-feature branch from 8802593 to 7330603 Compare May 25, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

[minor] remove/fix misleading "html_reports" feature from documentation

3 participants