search

mihirsoni / odfe-monitor-cli

Manage your Alerting monitors
Apache License 2.0
34 stars 22 forks source link

Backup full destination info #12

Closed kushagharahi closed 3 years ago

kushagharahi commented 3 years ago

This resolves #5. At some point in the future will have to go back and make destinations editable via the CLI too. Sorry about the reverts. I assume this PR will be squash and merged anyway

mihirsoni commented 3 years ago

@kushagharahi Thanks for the PR , few small comments. Overall could you run the formatter for better indentation ?

mihirsoni commented 3 years ago

Will this have any impact on existing destinations file ? Could you just verify before we merge this change ?

kushagharahi commented 3 years ago

@mihirsoni we're currently using this for production and the only thing this change will do is to populate the destinations.yml file with more values. I've verified it works

mihirsoni commented 3 years ago

@kushagharahi Thanks for confirming. Just to confirm after merging this PR the destinations.yml file will work as well ? I just want to ensure we are backward compatible :)

kushagharahi commented 3 years ago

@mihirsoni I just did this test: Create a destination.yml in the old style (id: name map) Try to push with the new code

No issues

Does that resolve your concern?

mihirsoni commented 3 years ago

Thanks for the confirmation @kushagharahi