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

Clean up git dependencies #2922

Closed
3 of 4 tasks
ggwpez opened this issue Jan 13, 2024 · 21 comments
Closed
3 of 4 tasks

Clean up git dependencies #2922

ggwpez opened this issue Jan 13, 2024 · 21 comments

Comments

@ggwpez
Copy link
Member

ggwpez commented Jan 13, 2024

Tasks

@davxy
Copy link
Member

davxy commented Jan 13, 2024

For bandernatch (cc @swasilyev):

@bkontur
Copy link
Contributor

bkontur commented Jan 13, 2024

there is also an issue for ethabi-decode here: paritytech/parity-bridges-common#2775
(afaik, Vincent will work on it)

cc: @vgeddes @claravanstaden

@vgeddes
Copy link
Contributor

vgeddes commented Jan 15, 2024

Ack, yes, I'll publish ethabi-decode and milagro-bls this week. Thanks for the reminder.

@vgeddes
Copy link
Contributor

vgeddes commented Jan 19, 2024

I've published them:

We will update our cumulus/bridges/snowbridge subtree next week which will include these changes.

@Morganamilo
Copy link
Contributor

From a crates perspective. Our tooling with auto strip optional git depends or switch git dependencies to crates.io if a compatible version exists. Meanwhile git deps with no crates.io releases are totally unpublishable. So releasing milagro_bls is high priority to me as it made some crates unpublishable on the last release.

@claravanstaden
Copy link
Contributor

@Morganamilo thanks for the info. As @vgeddes mentioned, the crates are published and I have updated their usages in this PR: #3029

@ggwpez
Copy link
Member Author

ggwpez commented Jan 25, 2024

@Morganamilo Which crates would still blocked after #3029 and by what?

@Morganamilo
Copy link
Contributor

Morganamilo commented Jan 25, 2024

That was the only crate I had issue with. Checking again, it was named milagro_bls on my end but I only see snowbridge-milagro-bls on crates.io. I guess the crate was renamed but that didn't make the release window and I got the old name?

@claravanstaden
Copy link
Contributor

Exactly, @Morganamilo.

@claravanstaden
Copy link
Contributor

Hey @Morganamilo! I was expecting the snowbridge crates to be updated with the polkadot-sdk 1.7.0 release, but it looks like the crates are still empty: https://crates.io/crates/snowbridge-pallet-inbound-queue/0.0.0

Will you let know if there is an issue with the publishing, or any other problem?

@ggwpez
Copy link
Member Author

ggwpez commented Feb 8, 2024

I think the crate publishing takes a while - we are also rate limited by crates-io. You can check here if stuff is being published https://crates.io/users/parity-crate-owner?sort=recent-updates

@Morganamilo
Copy link
Contributor

Is there anything blocking simple mermaid? Would moving to 0.1.1 work or do we need features that are unreleased?

@ggwpez
Copy link
Member Author

ggwpez commented Feb 13, 2024

Yea 0.1.1 should include my fix.
I will try to remove it here in the repo. PS: #3304

github-merge-queue bot pushed a commit that referenced this issue Feb 14, 2024
Re #2922

Changes:
- Use crates-io version of simple-mermaid as
[v0.1.1](https://github.com/glueball/simple-mermaid/releases/tag/v0.1.1)
supports no-std.
- Remove from `frame` since i did not see it used.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Feb 14, 2024

Done now @Morganamilo

@bkchr
Copy link
Member

bkchr commented Mar 11, 2024

We require some CI check that prevents adding git dependencies for crates that are already released.

milagro_bly from here

We should not care about this stuff. We should not release external crates.

@ggwpez
Copy link
Member Author

ggwpez commented Mar 11, 2024

We should not care about this stuff. We should not release external crates.

WDYM? Having git dependencies is bad for our tooling. So they should be banned in general.
Ah nvm. with "crates that are already released" you probably meant our crates, not external ones 👍

@bkchr
Copy link
Member

bkchr commented Mar 11, 2024

WDYM? Having git dependencies is bad for our tooling. So they should be banned in general.

I meant that the linked crate belongs to snowbridge. We as Parity should not release snowbride crates.

@ggwpez
Copy link
Member Author

ggwpez commented Mar 11, 2024

I meant that the linked crate belongs to snowbridge. We as Parity should not release snowbride crates.

Yes sure. That is why snowfork team is help us with this. Thanks Vincent and Clara.

@claravanstaden
Copy link
Contributor

We published that crates, yes. It will remain the Snowbridge team's responsibility.

bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
Re paritytech#2922

Changes:
- Use crates-io version of simple-mermaid as
[v0.1.1](https://github.com/glueball/simple-mermaid/releases/tag/v0.1.1)
supports no-std.
- Remove from `frame` since i did not see it used.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented May 15, 2024

It looks like litep2p is now being pulled in. @lexnv do you think it would be possible to use crates-io releases?
We can otherwise not properly release the sc-network crates that depend on it.

@ggwpez
Copy link
Member Author

ggwpez commented Jun 27, 2024

Going to close this since the remaining bandersnatch dep is feature gated and there not blocking the release process.
A CI check is also in place to prevent the addition of new git deps.

@ggwpez ggwpez closed this as completed Jun 27, 2024
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

No branches or pull requests

7 participants