Skip to content

Conversation

@Kaholaz
Copy link
Contributor

@Kaholaz Kaholaz commented Apr 19, 2025

This PR requires gleam-lang/hexpm-rust#43. Create a symlink named hexpm in the root of the project linking to my hexpm-rust branch to look at my changes. Cargo.toml needs to be cleaned before a merge.

Most changes are one of these three types of changes:

  1. Accommodating the hexpm-rust change that parses hexpm::version::Range.
  2. Changing imports to coincide with the 0.3 structure of pubgurb.
  3. Changes to the dependency resolution due to changes to the pubgrub::DependencyProvider trait.

Out of those, I believe nr. 3 warrants the most explanation (2. is explained in the hexpm-rust pull request -- let me know if anything is unclear). The major changes include choose_package_version of pubgrub::DependencyProvider being split into the functions choose_version and prioritize, and the trait requiring the the associated type Err: Error + 'static. The changes to the choose_package_version, might be the hardest one to get correct, but all tests are passing so I think we are good. The change to the associated Err type required a new error type and caused this to propagate a bit. I don't know if this is the best way to do it, but it does the job :D

Again: Let me know if anything is unclear or if you want me to write a more detailed changelog.

@Kaholaz Kaholaz force-pushed the bump-pubgrub branch 3 times, most recently from 36dc047 to 3e10ecf Compare May 1, 2025 11:34
@lpil
Copy link
Member

lpil commented May 9, 2025

I have published the new package as v4!

@Kaholaz
Copy link
Contributor Author

Kaholaz commented May 9, 2025

I have published the new package as v4!

@lpil nice :D I rebased and tested the changes.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Wonderful!! Thank you! I've left a couple notes inline

@Kaholaz Kaholaz force-pushed the bump-pubgrub branch 3 times, most recently from 9cf1c55 to 2241987 Compare May 12, 2025 14:16
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Hello! Thank you for this. I just published that new hexpm package, is there a small upgrade to do in this PR now that can be used?

@Kaholaz
Copy link
Contributor Author

Kaholaz commented Jun 4, 2025

@lpil Thank you; I have used the hexpm update to avoid a call to clone :D

@lpil
Copy link
Member

lpil commented Jun 10, 2025

Some linter issues to resolve!

This commit updates the pubgrub and hexpm dependency version. It also
makes sure that version ranges are parsed on creation rather than use to
improve error handling.
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Bravo!! Thank you very much!

@lpil lpil enabled auto-merge (rebase) June 16, 2025 12:38
@lpil lpil merged commit 7340d0a into gleam-lang:main Jun 16, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants