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

Replace hyperx with minimal, vendored Content-Range parsing #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

huonw
Copy link

@huonw huonw commented Jan 14, 2023

This removes the hyperx (https://github.com/dekellum/hyperx) dependency, because that dependencies takes the unusual approach of putting strict upper bounds on dependencies (not just restricting to semver-compatible versions, as Cargo does by default), and seems to be relatively unmaintained: a fix dekellum/hyperx#40 for that issue hasn't been merged for a while.

This dependency is only used to parse the Content-Range header value. This header is very simple, and thus it's easy to implement what's required directly in gha-toolkit.

Similar to #6, removing this dependency makes integrating gha-toolkit into other projects much easier. For example, in pantsbuild/pants#17840, I had use Cargo's [patch.crates-io] directive to refer to dekellum/hyperx#40, or else cargo couldn't find a valid set of dependencies (that is, I literally couldn't add gha-toolkit as a dependency due to hyperx).

This also positively improves the transitive dependencies of gha-toolkit too:

  • language-tags and matches are no longer required (i.e. the dependency tree/compile times is smaller)
  • bytes, form_urlencoded, idna, percent-encoding, url can use newer versions, since hyperx's upper bounds have disappeared

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.

1 participant