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

Update roxmltree requirement from 0.18 to 0.19 #1062

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Nov 20, 2023

Updates the requirements on roxmltree to permit the latest version.

Changelog

Sourced from roxmltree's changelog.

[0.19.0] - 2023-11-18

Changed

  • xmlparser is no longer a dependency and its fork is used internally.
  • ~5% faster parsing.
  • Fallback to Rc when Arc isn't available.
  • Bump MSRV to 1.60
  • Bump edition to 2021
  • Error variants have changed quite a lot.
  • XML declaration validation was simplified. We no longer check for attributes content. Meaning that version, encoding and standalone can contain any value now. But we still do check attribute names and order. And while we did validated those attributes before, they weren't really affecting the parser in any way. Therefore the parsing behavior is mostly unchanged.

Fixed

  • ParsingOptions::allow_dtd = false would not trigger an error when an empty DTD was present.

Removed

  • The xmlparser dependency.

[0.18.1] - 2023-09-30

Added

Fixed

  • Replace \r in CDATA as well.
  • no_std build. Thanks to @​wenyuzhao

[0.18.0] - 2023-02-04

Added

  • StringStorage that exposes an internal string storage.
  • Allocated strings are stored as Arc<str> and not String now.
  • Node::text_storage
  • Node::tail_storage
  • Attribute::value_storage
  • Node::range

Removed

  • Node::position. Use Node::range instead.

Fixed

[0.17.0] - 2023-01-06

Added

... (truncated)

Commits
  • 9f9df0a Version bump.
  • f37f358 Bump edition to 2021.
  • 28ad9e5 Simplify XML declaration validation.
  • 7e739cb A nicer unexpected XML declaration error.
  • 35016da Fix text parsing with a non-ASCII encoding.
  • 8d7c362 Rename lifetimes.
  • ce1d004 Fix build without default features.
  • 97737a1 Merge xmlparser into this crate.
  • 30b8c81 Simplify parser a bit.
  • 160e514 Bump MSRV to 1.60
  • Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added the dependencies PRs updating dependencies (dependabot automation) label Nov 20, 2023
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdnative/pr-1062

@chitoyuu
Copy link
Contributor

It doesn't seem like the update should affect our usage judging from the changelog.

bors r+

@chitoyuu
Copy link
Contributor

Seems like the the public bors instance is gone. I guess it's time to see what it takes to move to Github's merge queues.

@Bromeon
Copy link
Member

Bromeon commented Nov 21, 2023

Here's the official bors statement 😁

THIS APP IS NO LONGER RUNNING


Merge queues are not too involved, see godot-rust/gdext@164c48b. I could give it a try this week if you want 🙂

There are a few things to consider:

  1. There is no such thing as bors try anymore.
    • As a result, I made the minimal-ci slightly more extensive but still not as big as full-ci. The sweet spot is CI that runs in 3-6min but still covers 90% of common problems, so it's a matter of prioritizing jobs that are likely to catch issues.
    • This is purely a QoL thing. Since the full CI will still run eventually, we won't miss issues, but it might be that at the time of merging, a PR is unexpectedly blocked and needs additional rework.
  2. Both minimal-ci and full-ci must pass.
    • Merge queues only launch once the PR's own CI is green.
    • This means that if something is fixed on master in the meantime, a PR will need to be rebased/merged. This can be done via UI button for convenience.
    • It also means that the minimal time from opening a PR until merging is longer. While it was possible to write bors r+ in the initial PR message and have the PR merged ASAP, it now requires at least a full minimal-ci and full-ci run. It's not really problem though, one can pre-plan merging (called "auto merge").

In practice, merge queues work really well though. I've merged dozens of PRs without issues.

Also, not having to use bors try actually reduced manual intervention on my end. Most PRs that pass minimal-ci also pass the full one, so I have decent confidence when seeing that the PR checks are green.

@chitoyuu
Copy link
Contributor

Merge queues are not too involved, see godot-rust/gdext@164c48b. I could give it a try this week if you want 🙂

That sounds wonderful!

Also, not having to use bors try actually reduced manual intervention on my end. Most PRs that pass minimal-ci also pass the full one, so I have decent confidence when seeing that the PR checks are green.

I was concerned about losing bors try but it's nice to hear that you've had a positive experience without it. I think I might've been a little guilty of using it over-zealously in the past as well.

@Bromeon
Copy link
Member

Bromeon commented Nov 22, 2023

Merged first PR with merge queues ⚙️
The actual changes are in this commit: ac5f133

For now I only enabled merge queues and backported some doc fixes in PR #1063.

As a follow-up, you may want to copy more jobs from full-ci to minimal-ci. Good candidates might be the tests that run with features enabled; I wouldn't copy the nightly ones (apart from clippy, do they often catch issues that stable wouldn't)?


Last, there is now a new button to rebase a branch via UI. This may be necessary for existing PRs to pick up CI changes on master. Concretely, the new CI setup now expects a ci-status job to be successful (configured via UI). I'll do it for this one.

Note that this is update is generally not required, but here we have changes in master that are needed for minimal-ci to pass. In contrast to the previous setup, full-ci will not start if minimal-ci is red.

grafik

Updates the requirements on [roxmltree](https://github.com/RazrFalcon/roxmltree) to permit the latest version.
- [Changelog](https://github.com/RazrFalcon/roxmltree/blob/master/CHANGELOG.md)
- [Commits](RazrFalcon/roxmltree@v0.18.0...v0.19.0)

---
updated-dependencies:
- dependency-name: roxmltree
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
@Bromeon Bromeon force-pushed the dependabot/cargo/roxmltree-0.19 branch from 14c8acc to 418ed08 Compare November 22, 2023 20:56
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdnative/pr-1062

@chitoyuu
Copy link
Contributor

@Bromeon That was really quick. Thanks!

As a follow-up, you may want to copy more jobs from full-ci to minimal-ci. Good candidates might be the tests that run with features enabled; I wouldn't copy the nightly ones (apart from clippy, do they often catch issues that stable wouldn't)?

Moving nightly clippy to minimal-ci sounds like a good idea to me. New lints are getting added to clippy all the time, and a lot of them eventually land into stable. I remember there being cases where PRs got blocked despite passing minimal-ci because exactly that happened between the PR was opened and merged. It'd be good for contributors to be able to get instant feedback on that, especially since there isn't a try equivalent now.

I'll see what can be done.

@chitoyuu chitoyuu added this pull request to the merge queue Nov 23, 2023
Merged via the queue into master with commit b5d8960 Nov 23, 2023
7 checks passed
@chitoyuu chitoyuu deleted the dependabot/cargo/roxmltree-0.19 branch November 23, 2023 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies PRs updating dependencies (dependabot automation)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants