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

Replace subspace-runtime with subspace-fake-runtime-api #187

Merged
merged 1 commit into from
May 3, 2024

Conversation

dastansam
Copy link
Member

@dastansam dastansam commented May 3, 2024

companion PR autonomys/subspace#2735

Before:

Building [===========>            ] 708/1336:

Now:

 Building [=====================> ] 1257/1268

@dastansam dastansam requested a review from nazar-pc May 3, 2024 09:48
@dastansam dastansam self-assigned this May 3, 2024
Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Pushed commit that removes genesis config usage, please rebase and squash once upstream PR is updated

@nazar-pc
Copy link
Member

nazar-pc commented May 3, 2024

BTW this PR should proably be updated once upstream PR is merged, so we have commit from main here. I understand why you did it the way you did though.

@dastansam
Copy link
Member Author

Pushed commit that removes genesis config usage, please rebase and squash once upstream PR is updated

thanks, this is a lot better! couldn't think of using () 🤦

BTW this PR should proably be updated once upstream PR is merged, so we have commit from main here. I understand why you did it the way you did though.

yes, I would update the commit hash here once that PR was merged.

@nazar-pc
Copy link
Member

nazar-pc commented May 3, 2024

thanks, this is a lot better! couldn't think of using () 🤦

That generic is meaningful for legacy API that is soon to be removed from Substrate. When decoding from JSON it is not involved in anything and makes no difference, hence () is fine there and we'll likely remove it in the next Substrate upgrade anyway once it is gone upstream (they promised to remove in March, but it was still in the codebase).

@dastansam dastansam changed the title Remove subspace-runtime dependency and use subspace_service::fake_runtime Remove subspace-runtime dependency and use subspace-fake-runtime-api May 3, 2024
@dastansam dastansam requested a review from nazar-pc May 3, 2024 12:08
@dastansam dastansam changed the title Remove subspace-runtime dependency and use subspace-fake-runtime-api Replace subspace-runtime with subspace-fake-runtime-api May 3, 2024
Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

The impact of this should be even more than it seems because build step of subspace-runtime was compiling a lot of stuff, now all of it is gone. Thanks!

Cargo.toml Outdated Show resolved Hide resolved
Remove runtime genesis config usage

Update subspace commit hash
@nazar-pc nazar-pc enabled auto-merge May 3, 2024 12:24
@nazar-pc nazar-pc merged commit 7e57203 into main May 3, 2024
8 checks passed
@nazar-pc nazar-pc deleted the subspace-fake-runtime branch May 3, 2024 12:40
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