Skip to content

fonts: Replace deprecated xml-rs with [email protected]#41134

Merged
mrobinson merged 5 commits intoservo:mainfrom
Ajay-singh1:crate-change
Dec 9, 2025
Merged

fonts: Replace deprecated xml-rs with [email protected]#41134
mrobinson merged 5 commits intoservo:mainfrom
Ajay-singh1:crate-change

Conversation

@Ajay-singh1
Copy link
Copy Markdown
Contributor

@Ajay-singh1 Ajay-singh1 commented Dec 8, 2025

This PR updates xml-rs crate. On the official crate site it is mentioned to change the name of the crate with xml and update it.It is also explicitly mentioned that their won't be any code changes so I think their isn't any major API changes.

Reference:- https://crates.io/crates/xml-rs/0.8.28

Testing: It doesn't require testing because this is just the change in the crate's name

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 8, 2025
@Ajay-singh1
Copy link
Copy Markdown
Contributor Author

cc:- @mrobinson

@delan
Copy link
Copy Markdown
Member

delan commented Dec 8, 2025

please read our AI contributions policy before contributing.

@Ajay-singh1
Copy link
Copy Markdown
Contributor Author

Thanks! @delan. I think using AI isn't that bad anyways and for this very minor change it wouldn't have to be a very big issue. I have updated the PR description anyways.

@delan
Copy link
Copy Markdown
Member

delan commented Dec 8, 2025

thanks, the new description is much better :)

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 8, 2025
@delan delan enabled auto-merge December 8, 2025 11:48
@Taym95
Copy link
Copy Markdown
Member

Taym95 commented Dec 8, 2025

Thanks! @delan. I think using AI isn't that bad anyways and for this very minor change it wouldn't have to be a very big issue. I have updated the PR description anyways.

it’s not about using AI being bad or not, we have a policy in place and we should respect it!

@Ajay-singh1
Copy link
Copy Markdown
Contributor Author

Ajay-singh1 commented Dec 8, 2025 via email

@Taym95
Copy link
Copy Markdown
Member

Taym95 commented Dec 8, 2025

I'll take care of it.

On Mon, 8 Dec, 2025, 6:05 pm Taym Haddadi, @.> wrote: Taym95 left a comment (servo/servo#41134) <#41134 (comment)> Thanks! @delan https://github.com/delan. I think using AI isn't that bad anyways and for this very minor change it wouldn't have to be a very big issue. I have updated the PR description anyways. it’s not about using AI being bad or not, we have a policy in place and we should respect it! — Reply to this email directly, view it on GitHub <#41134 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AI7B2GCIBDDYAPFKT4RFPJL4AVV7VAVCNFSM6AAAAACOLUFSHKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMMRWG4YTENRVGI . You are receiving this because you authored the thread.Message ID: @.>

take care of what?

@Ajay-singh1
Copy link
Copy Markdown
Contributor Author

Ajay-singh1 commented Dec 8, 2025 via email

@delan delan added this pull request to the merge queue Dec 8, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 8, 2025
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Dec 8, 2025
@mrobinson
Copy link
Copy Markdown
Member

error[E0004]: non-exhaustive patterns: `xml::reader::XmlEvent::Doctype { .. }` not covered
   --> components/fonts/platform/freetype/android/xml.rs:21:15
    |
 21 |         match result? {
    |               ^^^^^^^ pattern `xml::reader::XmlEvent::Doctype { .. }` not covered
    |
note: `xml::reader::XmlEvent` defined here
   --> /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/xml-1.2.0/src/reader/events.rs:14:1
    |
 14 | pub enum XmlEvent {
    | ^^^^^^^^^^^^^^^^^
...
114 |     Doctype {
    |     ------- not covered
    = note: the matched value is of type `xml::reader::XmlEvent`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
    |
 44 ~             StartDocument { .. } | EndDocument | ProcessingInstruction { .. } | Comment(..) => {},
 45 ~             xml::reader::XmlEvent::Doctype { .. } => todo!(),
    |

This seems to need changes in order to compile against the newer version of xml.

@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Dec 9, 2025
Signed-off-by: Ajay Singh <[email protected]>
@Ajay-singh1
Copy link
Copy Markdown
Contributor Author

@mrobinson Fixed it.

@mrobinson mrobinson added the T-android Do a try run on Android label Dec 9, 2025
@github-actions github-actions bot removed the T-android Do a try run on Android label Dec 9, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2025

🔨 Triggering try run (#20057489980) for Android

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2025

✨ Try run (#20057489980) succeeded.

Copy link
Copy Markdown
Member

@mrobinson mrobinson 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, with one small change.

@servo-highfive servo-highfive added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Dec 9, 2025
@mrobinson mrobinson changed the title Replace deprecated xml-rs with xml crate (1.1) fonts: Replace deprecated xml-rs with [email protected] Dec 9, 2025
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Dec 9, 2025
@Ajay-singh1
Copy link
Copy Markdown
Contributor Author

Done!

@Ajay-singh1 Ajay-singh1 requested a review from mrobinson December 9, 2025 11:46
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Dec 9, 2025
@mrobinson mrobinson enabled auto-merge December 9, 2025 12:16
Signed-off-by: Ajay Singh <[email protected]>
auto-merge was automatically disabled December 9, 2025 13:57

Head branch was pushed to by a user without write access

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Dec 9, 2025
@mrobinson mrobinson enabled auto-merge December 9, 2025 14:08
@mrobinson mrobinson added this pull request to the merge queue Dec 9, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 9, 2025
Merged via the queue into servo:main with commit bc931c7 Dec 9, 2025
32 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Dec 9, 2025
@Ajay-singh1 Ajay-singh1 deleted the crate-change branch December 10, 2025 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants