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

feat: Make rug-backend optional for mithril-client crate #1884

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

Mr-Leshiy
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy commented Aug 8, 2024

Content

To allow disable LGPL usage which discussed in, added rug-backend feature for mithril-client and mithril-common crates.

Issue(s)

Closes #1786

@Mr-Leshiy
Copy link
Contributor Author

About the CI failure.
I've run this hash_computation_is_quicker_with_a_full_cache test locally and it passes.
Also seems to me that what testing under this test is not actually related to my changes.
So to be honest dont know what to do with it.

Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Mr-Leshiy, thank you for the contribution!

Before we can merge your PR, could you?

  • tidy up a bit the commit history as we don't squash commits when merging
  • add an entry in the CHANGELOG.md file at the root of the repository for this new feature
  • bump the patch versions of mithril-client, mithril-common and mithril-stm

Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@Alenar Alenar left a comment

Choose a reason for hiding this comment

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

LGTM.

I would have really liked to avoid using default-features = false for the mithril-common import in mithril-client, but as much as I can scratch my head there's no acceptable alternatives that come in mind.

@Mr-Leshiy Mr-Leshiy force-pushed the fix/lgpl-licence branch 3 times, most recently from d282bba to 62b8a90 Compare September 2, 2024 07:35
@Mr-Leshiy
Copy link
Contributor Author

Mr-Leshiy commented Sep 2, 2024

@jpraynaud @sfauvel @Alenar

Thanks for your feedback !

I've made requested changes for CHANGLELOG.md file, bumping versions and squashing commits.

@jpraynaud
Copy link
Member

@Mr-Leshiy actually your commits need to be signed in order to be merged in the main branch.
Can you re-push them with your signature?

@jpraynaud
Copy link
Member

Hi @Mr-Leshiy, on top of signing the commits, could you also sync your fork and make sure that the crates versions in your PR have not already been used by another PR in the mean time?

@Mr-Leshiy Mr-Leshiy force-pushed the fix/lgpl-licence branch 4 times, most recently from ebd225e to 677be42 Compare September 3, 2024 09:17
@Mr-Leshiy
Copy link
Contributor Author

@jpraynaud done 🚀

@jpraynaud
Copy link
Member

@jpraynaud done 🚀

Thanks @Mr-Leshiy, I guess that all the commits need to be signed and I see that only the last one is signed now.
Maybe a force push can force the signature of the commit 62b8a90.

We'll be able to merge the PR as soon as this is the case 🚀

…ommon and mithril-client crates

Signed-off-by: Mr-Leshiy <leshiy12345678@gmail.com>
Signed-off-by: Mr-Leshiy <leshiy12345678@gmail.com>
@jpraynaud jpraynaud merged commit ffc7943 into input-output-hk:main Sep 3, 2024
38 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.

Cargo Deny complains about LGPL-3.0 licenses
4 participants