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

Newly scaffolded chains should not import monitoringp module by default #2641

Closed
aljo242 opened this issue Jul 20, 2022 · 1 comment · Fixed by #2648
Closed

Newly scaffolded chains should not import monitoringp module by default #2641

aljo242 opened this issue Jul 20, 2022 · 1 comment · Fixed by #2648
Labels

Comments

@aljo242
Copy link
Contributor

aljo242 commented Jul 20, 2022

Describe the bug

By default, newly scaffolded chains import the monitoringp module. I believe that this should be an "opt-in" import only for users who are going to be using this feature. The cli should give developers flexibility and start them off in the most lightweight point possible. I think the best course of action would be to have a prompt ask developers something like "would you like this chain to connect to Ignite Network" or something similar, making it an "opt-in" feature.

To reproduce
Steps to reproduce the behavior:

  1. Scaffold a new chain and inspect the imported modules in app.go

What version are you using?

Ignite CLI version:     development
Ignite CLI build date:  2022-07-20T16:30:02
Ignite CLI source hash: fe175c961f9061d8d3aadfe9f8c2bcde9cfd6731
Your OS:                linux
Your arch:              amd64
Your go version:        go version go1.18.3 linux/amd64
Your uname -a:          Linux cozart-dev 5.18.10-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 07 Jul 2022 17:18:13 +0000 x86_64 GNU/Linux
Your cwd:               /home/cozart/go/src/github.com/aljo242/shmeechain
Is on Gitpod:           false

@aljo242 aljo242 added the report label Jul 20, 2022
@aljo242 aljo242 changed the title Newly scaffolded chains should not import monitoringp module Newly scaffolded chains should not import monitoringp module by default Jul 20, 2022
@lumtis
Copy link
Contributor

lumtis commented Jul 21, 2022

I'm in favor of removing it.
Even though the module is inactive when initialized with an empty genesis state, it may be confusing for developers to see their app scaffolded with a non-used module.
We will not need it for the time being, and at this point, we can hope the new module wiring design will be out so we can implement a reliable import command.
Also, I don't think it is complex to import the module manually for edge cases it's needed.

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 a pull request may close this issue.

2 participants