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

feat: cosmwasm plugin #2

Closed
wants to merge 43 commits into from
Closed

Conversation

yerasyla
Copy link

No description provided.

@jeronimoalbi jeronimoalbi changed the title Feat/cosmwasm plugin feat: cosmwasm plugin Sep 1, 2023
@jeronimoalbi
Copy link
Member

Bounty referece: #56

…nary instead of reading them from the filesystem
@yerasyla
Copy link
Author

Thanks for the feedback @Pantani , I've addressed most of the issues by applying .plush files, genny generator, and placeholder code. I would like to get feedback on current solution for maintaining app.go file.

@Pantani
Copy link
Collaborator

Pantani commented Oct 13, 2023

@yerasyla, thanks for the changes and efforts. I think you didn't understand the idea; you should inject the code into the existing templates we already have in the code. You can use the built-in ast package or the placeholder we already have in the code. The last implementation was more straightforward than this one; there are too many files to maintain now.

@yerasyla
Copy link
Author

yerasyla commented Nov 7, 2023

@Pantani @jeronimoalbi I have attempted to use ASTs to modify existing app.go file that was scaffolded with Ignite CLI. There is no need to maintain separate app.go now. Could you provide high level feedback on what direction I should focus to deliver the best possible Ignite app for cosmwasm integration.

@Ehsan-saradar
Copy link
Contributor

Hey @yerasyla Thanks for the great work! I'm new to ignite and i'm very interested to know how you implemented your AST-based approach for modifying the existing go files in chain. Could you give me a brief on how it works? How flexible is your approach (e.g. If some of the app.go file is changed manually would it still work as intended?) or if there are any situations where it can not work properly?

@yerasyla
Copy link
Author

Hey @yerasyla Thanks for the great work! I'm new to ignite and i'm very interested to know how you implemented your AST-based approach for modifying the existing go files in chain. Could you give me a brief on how it works? How flexible is your approach (e.g. If some of the app.go file is changed manually would it still work as intended?) or if there are any situations where it can not work properly?

Hi @Ehsan-saradar , thank you for feedback! Provided AST based approach basically consist of:

  1. Identifying the location in AST representation of the file, where you want to inject code with ast.Inspect()
  2. The next step is injecting the code, usually with append() command

It took me a while to figure it, and the current implementation could be heavily optimised. But if you want more insights maybe we can have a quick call and discuss ASTs.

Regarding flexibility, I believe it depends on the severity of changes in file. E.g. some code injections depend on the name of expression, if the expression is renamed or deleted, it can no longer be found in AST and there will be no injection. I believe it is still flexible and with some optimisation it can handle most edge cases when it comes to change in app.go.

Also this tool helped me a lot in visualising AST of the files: https://astexplorer.net/

@michelleellen
Copy link

Hi @yerasyla thanks for your contributions. For the bounty, could you contact us at hello@tendermint.com? Thank you!

@yerasyla
Copy link
Author

Hi @yerasyla thanks for your contributions. For the bounty, could you contact us at hello@tendermint.com? Thank you!

Hello, I have just send the mail from yerasyl.amanbek@alumni.nu.edu.kz , could you check. Thank you!

@Pantani Pantani closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants