Align html5ever rcdom dependencies (#278) - #279
Conversation
Upgrade `html5ever` and `markup5ever_rcdom` onto the 0.39 parser stack so both crates use the same `markup5ever` trait definitions. Pin `markup5ever_rcdom` to the 0.39 line because the compatible crate is published as `0.39.0+unofficial`, while Cargo ignores build metadata in manifest requirements.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
💤 Files with no reviewable changes (1)
Resolves version conflict between html5ever and markup5ever_rcdomThis PR resolves issue Dependencies
Documentation
Testing
Test infrastructure refactoringReplaced the broad ValidationAll checks passed: format validation, clippy linting with WalkthroughUpdate two crates so they share the same markup5ever parser stack: ChangesParser upgrade, docs, and test harness changes
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAligns the crate’s HTML parsing stack by upgrading html5ever to 0.39.0 and pinning markup5ever_rcdom to 0.39.0, with Cargo.lock regenerated to ensure a consistent set of html5ever/markup5ever/xml5ever-related dependencies. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider adding a brief comment in Cargo.toml explaining why markup5ever_rcdom is pinned to an exact version (=0.39.0), so future maintainers understand the coupling requirement with html5ever and avoid accidental desynchronization.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a brief comment in Cargo.toml explaining why markup5ever_rcdom is pinned to an exact version (=0.39.0), so future maintainers understand the coupling requirement with html5ever and avoid accidental desynchronization.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c2063aad6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Line 26: Replace the exact version pin for the dependency named
markup5ever_rcdom in Cargo.toml: change the requirement from "=0.39.0" to a
caret requirement "^0.39.0" so Cargo will accept future 0.39.x patches (it still
resolves to 0.39.0+unofficial due to build metadata); if you intentionally must
keep the exact pin, leave it but add an inline comment next to markup5ever_rcdom
explaining the specific reason for the exception to the caret mandate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1c80ab7c-b8d2-454d-9a22-321624774572
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
Cargo.toml
Remove the module-level `unfulfilled_lint_expectations` allowance from the integration test prelude. Use item-level `unused_imports` allowances for the shared re-exports because each integration test compiles the prelude independently and consumes a different subset of those utilities.
Add a manifest comment explaining why `markup5ever_rcdom` is pinned to match the `html5ever` parser stack. This makes the dependency coupling visible before future updates change either crate independently.
Replace the exact `markup5ever_rcdom` pin with the repository-standard caret requirement so compatible 0.39 patch updates remain available. Keep the coupling comment because `RcDom` still needs to track the same parser stack as `html5ever`.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/prelude/mod.rs`:
- Around line 5-10: Replace the forbidden #[allow(unused_imports)] attributes on
the re-exports in tests/prelude/mod.rs with narrowly scoped
#[expect(unused_imports, reason = "...")] annotations: apply one expect per
re-export line (e.g., for pub use assert_cmd::{Command, prelude::*};, pub use
predicates::prelude::*; and pub use rstest::{fixture, rstest};) and add an
expect on the common module declaration if present; in each reason string state
why the import is intentionally re-exported for test ergonomics (e.g.,
"re-exported for test helpers and macros used across multiple test modules") so
the lint suppression is justified and compliant with the coding guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3540730b-a877-432c-9238-1268054e071a
📒 Files selected for processing (2)
Cargo.tomltests/prelude/mod.rs
Remove the integration test prelude module and import only the helpers each consumer actually uses. Keep shared macros wired through direct `common` module paths so tests no longer need broad prelude re-exports or unused-import suppression.
Document the `html5ever` and `markup5ever_rcdom` version relationship in the architecture and developer guides. Add a trybuild pass test that compiles the `RcDom` parser sink with the active `html5ever` parser stack so future dependency bumps catch trait bound regressions at compile time.
Summary
This branch resolves issue #278 by moving
html5everandmarkup5ever_rcdomonto the same 0.39 parser stack. That keepsRcDomimplementing theTreeSinktrait expected byhtml5ever0.39 and removes the duplicate
markup5evertrait-bound conflict.Closes #278.
Review walkthrough
html5ever,markup5ever,markup5ever_rcdom, andxml5everstack.Validation
make check-fmt: passed.make lint: passed withcargo clippy --all-targets --all-features -- -D warnings.make test: passed withRUSTFLAGS="-D warnings" cargo test --all-targets --all-features.coderabbit review --agent: passed with zero findings.Summary by Sourcery
Build: