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

mock: Switch out xml-builder with quick-xml #20

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

ColinKinloch
Copy link
Contributor

Last week xml-builder went from 0.5.3 to 0.5.4 making API changes which stopped CI from passing.

On crates.io xml-builder has 7 dependents vs quick-xmls 650.

This currently passes cargo test, however more should probably be done to verify this change doesn't produce different XML.

This MR is on top of #19

@ColinKinloch
Copy link
Contributor Author

ColinKinloch commented Feb 6, 2025

  • Tested with each subcommand of cargo run --example obsapi.
  • Tested with obsctl using cargo run -- monitor
  • obs-gitlab-runner tests pass when depending on using open-build-service-mock from this branch.

obs-gitlab-runner fails to compile with open-build-service-api from this branch.
obs-gitlab-runner tests fails to compile with open-build-service-api from this branch. Which can be fixed by bumping the obs-glr tokio version to 1.35.0.

@em- em- self-assigned this Feb 7, 2025
@@ -102,8 +101,7 @@ impl Respond for ProjectBuildCommandResponder {
// error is wrapped *as a string* inside of a different
// error. Mimic the behavior here.
let inner_xml = unknown_package(package_name.to_owned()).into_xml();
let mut inner = Vec::new();
inner_xml.render(&mut inner, false, true).unwrap();
let inner = inner_xml.into_inner().into_inner();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just checking: is the double into_inner() needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes: quick_xml::Writer<Cursor<Vec<u8>>> -> Cursor<Vec<u8>> -> Vec<u8>.

@em- em- added this pull request to the merge queue Feb 7, 2025
Merged via the queue into collabora:main with commit 6be987e Feb 7, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants