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

schema: add multifundchannel generation #7215

Merged

Conversation

daywalker90
Copy link
Contributor

@daywalker90 daywalker90 commented Apr 13, 2024

Using the FundChannel ChannelTypeName override for this aswell.
This command actually was the only one with the following edge case when generating the camel case name for MultifundchannelChannel_idsChannel_type. Since it splitted at _ the camel case in idsChannel was removed by .title(). So i changed the to_camel_case function to only uppercase the first character of a word and not implicitly lowercase all other letters.

Fixes: #6193

an edge case like MultifundchannelChannel_idsChannel_type
was previously converted to MultifundchannelChannelIdschannelType
instead of the correct MultifundchannelChannelIdsChannelType
@cdecker
Copy link
Member

cdecker commented Apr 14, 2024

Oh yeah, those generated type names 🤦🏼‍♂️ I am hoping that at some point we move away from JSON schema as it doesn't really lend itself to composition and tends to define stuff in nested structures, which make this kind of name spacing necessary. I'd rather like to have a protobuf-like format where we start by defining small messages and then complex ones by composing them from the simpler ones. Then we could give meaningful names to these leave types as well.

I'm sorry for all the pain that msggen has been 😅

Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

Excellent find 👍

ACK ddd4ffc

@cdecker
Copy link
Member

cdecker commented Apr 17, 2024

Let's start merging so your stack doesn't grow too high :-)

@cdecker cdecker merged commit 58b78d2 into ElementsProject:master Apr 17, 2024
35 checks passed
@daywalker90
Copy link
Contributor Author

Let's start merging so your stack doesn't grow too high :-)

yes please

@daywalker90 daywalker90 deleted the multifundchannel-schema-fix branch April 24, 2024 10:47
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.

gRPC does not support batch open?
2 participants