Skip to content

Conversation

Pantani
Copy link
Collaborator

@Pantani Pantani commented Apr 4, 2024

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

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
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
@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
* 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
* 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
@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
Projects
None yet
3 participants