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.

Sign up for GitHub

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

Jump to bottom

feat: support custom proto path #4071

Merged
merged 32 commits into from
Apr 20, 2024
Merged

feat: support custom proto path #4071

merged 32 commits into from
Apr 20, 2024

Conversation

Pantani
Copy link
Collaborator

@Pantani Pantani commented Apr 4, 2024 •
edited
Loading

close #3979
close #4007

Description

Create a proto-dir flag for scaffold commands to use a different proto directory. This PR also manages the buf.work.yaml to be populated automatics depending on your proto directory

@Pantani Pantani self-assigned this Apr 4, 2024
Pantani and others added 21 commits April 4, 2024 22:45
julienrbrt
julienrbrt requested changes Apr 11, 2024
View reviewed changes
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

changes lgtm but I don't understand the use case tbh.
We should actually move towards having protos directly hosted in the modules instead.
like f.e: https://github.com/cosmos/cosmos-sdk/tree/main/x/bank/proto
this feature won't less us easily do that later on.

@Pantani
Copy link
Collaborator Author

Pantani commented Apr 12, 2024

changes lgtm but I don't understand the use case tbh. We should actually move towards having protos directly hosted in the modules instead. like f.e: https://github.com/cosmos/cosmos-sdk/tree/main/x/bank/proto this feature won't less us easily do that later on.

@julienrbrt, We already support a custom proto path in the chain's config.yaml, but if you do this, you can't scaffold anymore because the scaffold doesn't support the custom proto path. But I agree regarding having one proto folder for each module

@Pantani Pantani requested a review from julienrbrt April 12, 2024 12:36
@Pantani
Copy link
Collaborator Author

Pantani commented Apr 12, 2024

changes lgtm but I don't understand the use case tbh. We should actually move towards having protos directly hosted in the modules instead. like f.e: https://github.com/cosmos/cosmos-sdk/tree/main/x/bank/proto this feature won't less us easily do that later on.

This feature can also be changed in the future so we can support a custom module dir inside the module folder

@julienrbrt
Copy link
Member

changes lgtm but I don't understand the use case tbh. We should actually move towards having protos directly hosted in the modules instead. like f.e: https://github.com/cosmos/cosmos-sdk/tree/main/x/bank/proto this feature won't less us easily do that later on.

This feature can also be changed in the future so we can support a custom module dir inside the module folder

if you think it is worth it to support that level of customizability, then fine 👍🏾

julienrbrt
julienrbrt previously approved these changes Apr 12, 2024
View reviewed changes
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

tACK!

@Pantani Pantani enabled auto-merge (squash) April 17, 2024 11:06
@Pantani Pantani requested a review from julienrbrt April 18, 2024 13:40
@Pantani Pantani merged commit 6364ecb into main Apr 20, 2024
46 checks passed
@Pantani Pantani deleted the feat/custom-proto-path branch April 20, 2024 01:04
julienrbrt pushed a commit that referenced this pull request May 29, 2024
@Pantani @julienrbrt
feat: support custom proto path (#4071)
053903f 
* support custom proto path

* add changelog

* check buf work proto

* check if the buf.work.yaml has the same proto path from the config file

* add comments

* rmeove unused code

* remove unused proto paths and add integrations tests

* fix resolve includes function

* fix integration tests

* scaffold a list into the tests

* proto dir flag

* avoid read config for scaffold commands

* fix wrong embed path

* scaffold a chain with a custom proto path

* fix proto-dir flag

* fix protoDir var for plush files

* fix buf pkg

* fix unit tests for buf files

* add proto dir to scaffold chain

* fix wrong prodir for the module

* fix wrong proto dir

* fix text scaffold conflits

* fix wront proto path

---------

Co-authored-by: Pantani <Pantani>
@Pantani Pantani added the backport/v28.x.y Backport to v28.x.y label Jul 2, 2024
mergify bot pushed a commit that referenced this pull request Jul 2, 2024
@Pantani @mergify
feat: support custom proto path (#4071)
df756e1 
* support custom proto path

* add changelog

* check buf work proto

* check if the buf.work.yaml has the same proto path from the config file

* add comments

* rmeove unused code

* remove unused proto paths and add integrations tests

* fix resolve includes function

* fix integration tests

* scaffold a list into the tests

* proto dir flag

* avoid read config for scaffold commands

* fix wrong embed path

* scaffold a chain with a custom proto path

* fix proto-dir flag

* fix protoDir var for plush files

* fix buf pkg

* fix unit tests for buf files

* add proto dir to scaffold chain

* fix wrong prodir for the module

* fix wrong proto dir

* fix text scaffold conflits

* fix wront proto path

---------

Co-authored-by: Pantani <Pantani>
(cherry picked from commit 6364ecb)

# Conflicts:
#	go.mod
#	ignite/cmd/chain.go
#	ignite/cmd/cmd.go
#	ignite/cmd/scaffold.go
#	ignite/cmd/scaffold_chain.go
#	ignite/cmd/scaffold_configs.go
#	ignite/cmd/scaffold_message.go
#	ignite/cmd/scaffold_module.go
#	ignite/cmd/scaffold_package.go
#	ignite/cmd/scaffold_params.go
#	ignite/cmd/scaffold_query.go
#	ignite/config/chain/base/config.go
#	ignite/config/chain/parse.go
#	ignite/config/chain/v1/config.go
#	ignite/config/chain/v1/config_test.go
#	ignite/config/chain/v1/validator_servers.go
#	ignite/pkg/cosmosgen/generate.go
#	ignite/services/chain/proto.go
#	ignite/services/chain/serve.go
#	ignite/services/scaffolder/configs.go
#	ignite/services/scaffolder/init.go
#	ignite/services/scaffolder/message.go
#	ignite/services/scaffolder/module.go
#	ignite/services/scaffolder/packet.go
#	ignite/services/scaffolder/params.go
#	ignite/services/scaffolder/query.go
#	ignite/services/scaffolder/scaffolder.go
#	ignite/services/scaffolder/type.go
#	ignite/templates/app/files/proto/buf.gen.pulsar.yaml
#	ignite/templates/app/files/{{protoDir}}/buf.gen.pulsar.yaml
#	ignite/templates/app/files/{{protoDir}}/buf.gen.pulsar.yaml.plush
#	ignite/templates/app/proto.go
#	ignite/templates/module/create/configs.go
#	ignite/templates/module/create/options.go
#	ignite/templates/module/create/params.go
@mergify mergify bot mentioned this pull request Jul 2, 2024
Closed
@Pantani Pantani removed the backport/v28.x.y Backport to v28.x.y label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienrbrt julienrbrt julienrbrt approved these changes

@ilgooz ilgooz Awaiting requested review from ilgooz ilgooz is a code owner

@salmad3 salmad3 Awaiting requested review from salmad3 salmad3 is a code owner

@toschdev toschdev Awaiting requested review from toschdev toschdev is a code owner

@jeronimoalbi jeronimoalbi Awaiting requested review from jeronimoalbi jeronimoalbi is a code owner

Assignees

@Pantani Pantani

Labels
component:ci CI/CD workflow and automated jobs. component:cmd component:configs component:packages component:templates type:internal type:services Service-related issues.
Projects
None yet
Milestone
No milestone
3 participants
@Pantani @julienrbrt @ilgooz