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

Fix hydra-typegen to properly generate types for deprecated/deleted events & extrinsics #531

Merged

Conversation

zeeshanakram3
Copy link
Contributor

addresses Joystream/joystream#4877

based on #527

…port for deprecated events/call

affects: @joystream/hydra-typegen
Copy link
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Works as planned.

Its also possible for a module to be removed from latest runtime in which case extractCall and extractEvent will throw "No metadata found for module xyz".

Does it make sense to add support for missing modules now so we don't have to make it in future?

@zeeshanakram3
Copy link
Contributor Author

Its also possible for a module to be removed from latest runtime in which case extractCall and extractEvent will throw "No metadata found for module xyz".

Does it make sense to add support for missing modules now so we don't have to make it in future?

Good point. Addressed that in 3143361

@zeeshanakram3 zeeshanakram3 force-pushed the fix-typegen-for-deprecated-events branch from 3143361 to 84243e8 Compare September 26, 2023 12:53
@mnaamani mnaamani self-requested a review September 26, 2023 13:04
Copy link
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Great stuff, just minor fix for linter and this is good to go!

@@ -136,34 +136,74 @@ types don't much the metadata definiton`,
strict: flags.strict,
} as IConfig
}

async buildGeneratorConfigs(config: IConfig): Promise<GeneratorConfig[]> {
async buildGeneratorConfigs(config: IConfig): Promise<{
Copy link
Member

Choose a reason for hiding this comment

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

Just need an extra line here to satisfy linter ;)

https://github.com/Joystream/hydra/pull/531/checks#step:4:365

Suggested change
async buildGeneratorConfigs(config: IConfig): Promise<{
async buildGeneratorConfigs(config: IConfig): Promise<{

Copy link
Member

Choose a reason for hiding this comment

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

you can do it manually with yarn lint --fix

@mnaamani mnaamani self-requested a review September 26, 2023 13:25
@mnaamani mnaamani merged commit a70c88e into Joystream:master Sep 26, 2023
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants