Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix: Upgrade Kinobi to Codama #39

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

aoikurokawa
Copy link
Collaborator

Kinobi renamed to Codama.
This PR upgrades to Codama, one of the merit is that we can specify traitOption.

@aoikurokawa aoikurokawa marked this pull request as ready for review December 28, 2024 04:45
Copy link
Collaborator

@coachchucksol coachchucksol left a comment

Choose a reason for hiding this comment

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

The upgrade looks good to me, but what is the purpose?

@aoikurokawa
Copy link
Collaborator Author

aoikurokawa commented Jan 7, 2025

if we upgrade to to codama, we can have some merits. One thing that we can specify the trait of struct. I think this is useful.
For example, we write CLI, one of the command takes the enum value. Enum should be derived ValueEnum if we use clap crate. In this case, we can write like this in scripts/generate-client.js, then all enum is going to be derived from clap::ValueEnum

const traitOptions = {
    baseDefaults: [
        'borsh::BorshSerialize',
        'borsh::BorshDeserialize',
        'serde::Serialize',
        'serde::Deserialize',
        'Clone',
        'Debug',
        'Eq',
        'PartialEq',
    ],
    dataEnumDefaults: [],
    scalarEnumDefaults: ['Copy', 'Hash', 'num_derive::FromPrimitive', 'clap::ValueEnum'],
    structDefaults: [],
};

weightTableKinobi.accept(renderers.renderRustVisitor(path.join(rustWeightTableClientDir, "src", "generated"), {
    formatCode: true,
    crateFolder: rustWeightTableClientDir,
    deleteFolderBeforeRendering: true,
    toolchain: "+nightly-2024-07-25",
    traitOptions,
}));

@coachchucksol
Copy link
Collaborator

I appreciate your work here, however, I'm hesitant to introduce anything new here. So I'm closing - if it's not broke, dont fix it

Again, thank you for the work here

@coachchucksol
Copy link
Collaborator

NVM, upon further review, we'll merge!

@coachchucksol coachchucksol reopened this Feb 11, 2025
Copy link
Collaborator

@ebatsell ebatsell left a comment

Choose a reason for hiding this comment

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

Actually this is gonna break after jito-foundation/restaking#203 is merged, can hold off on addressing it until we have a good plan

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants