-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Angular2 webdriver test updates #2038
Conversation
@@ -1,12 +1,13 @@ | |||
#!/usr/bin/env python | |||
"""A vtctld2 webdriver test.""" | |||
"""A vtctld2 webdriver test that tests the different views of status page.""" |
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.
You can keep the old more generic description here.
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.
Done.
Also went ahead and checked in the compiled angular2 app into web/vtctld2/app. |
Josh, can you please write a minimal script to copy the compiled Angular 2 app and also add a Makefile target for it? This way it's clear how to update the app. |
A script isn't really necessary, running make build_web should do the trick as long as bootstrap_web.sh was run. |
LGTM but +1 to Michael's suggestion. Reviewed 4 of 4 files at r1, 3 of 5 files at r2, 1 of 3 files at r3, 15 of 16 files at r4, 1 of 1 files at r5. Comments from Reviewable |
Josh, maybe some context here: The idea is to decouple the checked in version of the app from the own you're currently developing and working one. The later one should not be under version control and it should not show up as a changed file when you rebuild it. Therefore, we'll need a script which copies the current state to a dedicated folder where the "release" version of the app lives. Does that make sense? |
Reviewed 3 of 3 files at r6. Comments from Reviewable |
Anthony said that we already have this distinction with the Just for the record: Which step in |
Ok so here is what I'm thinking the flow is for someone making changes to the web app:
👍 ? 👎 ? |
Review status: 21 of 23 files reviewed at latest revision, 2 unresolved discussions. tools/generate_web_artifacts.sh, line 14 at r7 (raw file):
Please make this conditional on the existance of the directory: if [[ -d $vtctld2_dir/app ]]; then
rm -rf $vtctld2_dir/app
fi Comments from Reviewable |
Review status: 21 of 23 files reviewed at latest revision, 2 unresolved discussions. tools/generate_web_artifacts.sh, line 14 at r7 (raw file):
|
@enisoc
This supersedes PR #1988. I'll work on potential Travis caching and/or Dockerizing in a later PR.