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

Updates to get the example in the README.md working #2

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

cowlicks
Copy link

@cowlicks cowlicks commented Apr 4, 2024

These changes were needed to get the example in the README.md working, and to silence existing warnings. There was only one existing test, and it passes.

The only thing that may be controversial here are these two places where I've patched dependencies, and point to my own fork. This is intended to be a temporary solution. The practice is common within this repo. User @Frando own several of these branches, but he is a more well known community member 😉.

  • this line points to a branch that includes this PR to libutp-sys which I made to fix a build failure in bindgen.
  • this line. Where we were using hypercore-protocol-rs v3.1 However building this version is now broken because of it's dependency on snow v0.7.0-alpha5 which causes a build failure. The branch this now points to contains this patch which bumps the snow version to one that works (0.8). I would like to merge this upstream but the hypercore-protocol-rs repository does not have a 3.1 tag or branch which I could make a PR against. A different fix could be bumping the hypercore-protocol-rs version itself. I'm looking into this.

cowlicks added 11 commits April 3, 2024 12:40
This dir does not exist so `cargo` would fail on every command.
The usage within the library was not compatible with the version listed in Cargo.toml.
So I updated, and reworked the code to the current Clap
I'm new to codebase so I don't feel comfortable removing these yet
 Please enter the commit message for your changes. Lines starting
Copy link
Member

@ttiurani ttiurani left a comment

Choose a reason for hiding this comment

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

Looks reasonable, good to have the example to at least work before doing anything else. Is the idea to upgrade hypercore-protocol to latest master eventually?

@cowlicks
Copy link
Author

cowlicks commented Apr 9, 2024

@ttiurani That is the plan! I'd like to use some of the functionality in here (in corestore and replicator) for adding integration tests to hyperbee

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