-
Notifications
You must be signed in to change notification settings - Fork 10
Merged Monitoring and Monitoring Service into one #32
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR is only removing monitoringService
from the cli, it is not merging the functionality previously provided by it into monitoring
The schematics should also be updated to merge |
@sooraj1002 Yes ill work on your reviewed suggestions and do the respective changes |
|
||
await createMonitor( | ||
await createskipDocker( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is workable, but this feature should ideally be handled in combination with changes in the schematics as well since the bootstrapping is just one part of the system.
What happens when a user wants to generate monitoring related files? they would need to run two commands, nest g monitor
and nest g monitorModule
whereas it should ideally be implemented something as nest g monitor --skipDocker
. The flag should be implemented at the level of each schematic, and while bootstrapping a new project, the --skipDocker
flag should be propagated to each command
Fixes/included in pr #34 |
Merge monitor and monitoringService commands in cli into one
First level implementation on skipDocker command
Fixes issue : Merge monitor and monitoringService commands #30
Does this PR introduce a breaking change?
Other information
(#30 (comment))