-
Notifications
You must be signed in to change notification settings - Fork 3
Add monitoring and some housekeeping #156
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
| install-monitor: | ||
| mkdir -p ./ahm-monitor/backend/data | ||
| cd ahm-monitor/backend \ | ||
| && npm install \ |
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.
&& instead of #!/usr/bin/env bash + set -e for brevity or why?
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.
&& really mainly for staying in same dir, just runs each line of a recipe with working directory as the root. I.e. here cd changes directory from the root to ahm-monitor/backend, then the next line is run in the root:
test:
mkdir tmp
cd tmp
touch testyou'd end up with the behaviour of touch ./test, rather than the expected touch ./tmp/test
You can fix it with a shebang so it runs it all in the same instance, but I slightly prefer this as the recipes are then consistent, rather than throwing a shebang into some recipes but not others. don't feel too strongly about it though, happy to change if you think it's better style
| @echo "just run-backend" | ||
| @echo "open https://migration.paritytech.io/?backend_url=http://localhost:3000" | ||
| # TODO @donal: Monitoring here | ||
| cd ahm-monitor/backend && yarn run start |
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.
Yarn here vs npm above? will it cause issues?
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.
yarn is enforced in the overall package.json, I found a way round it (it's basically just for tests) but forgot to revert this line
| rm -f zombie-bite/doppelganger/{.crates.toml,.crates2.json} | ||
|
|
||
| # Clean up everything. | ||
| clean-harder: clean |
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.
yeah man, I would love this :)) the repo still generates so many things that should be git-ignored on each run.
Started on top of #154 and some of those commits remain to avoid force-pushing.