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

chore(deps): simplify how some dependencies are fetched #221

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

noctisatrae
Copy link

@noctisatrae noctisatrae commented Jan 19, 2025

Summary of the changes

  • edited config.toml to use a git repository with a commit hash and a package specifier (telling Cargo where to look for stuff)
  • cargo.lock was updated when I pulled dependencies after my changes
  • updated instructions in the README.

Goals

  • Improve developer experience
  • According to the Rust standard, crates should include everything in order to be built

Hope this will be useful <3

@noctisatrae
Copy link
Author

I was browsing around and saw that #184 was in the work, feel free to close this if you don't need it then :)

@noctisatrae noctisatrae changed the title chore(deps): simplify how Malachite is included in the project chore(deps): simplify how some dependencies are fetched Jan 23, 2025
@noctisatrae
Copy link
Author

noctisatrae commented Jan 23, 2025

I watched the changes made in #225 and wanted to further simplify your pipeline. Now that Cargo handles the download of the repo & the version, your build actions should be much easier to write. I hope this is useful :)

EDIT: would you like me to make changes to the GitHub actions to delete the checkout steps? (and also the Dockerfile)

@christopherwxyz
Copy link

Malachite packages were published. Probably can close this PR.

@noctisatrae
Copy link
Author

You're right, my PR should be edited to account for the release of Malachite. However, some changes are still relevant today: eth-signature-verifier is still imported with a relative path.

I can understand this is not the priority ATM tho!

Cargo.toml Show resolved Hide resolved
@noctisatrae
Copy link
Author

noctisatrae commented Feb 6, 2025

This PR is a mess I need to maintain as the changes drop but I've been underwater lately ;)

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.

3 participants