-
Notifications
You must be signed in to change notification settings - Fork 6.8k
build: firebase integration for dashboard #5182
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
build: firebase integration for dashboard #5182
Conversation
* As part of the initial dashboard setup, this installs Angular Fire and Angular Material in the dashboard. * For testing purposes the list of all payload results will be printed right now.
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.
LGTM
tools/dashboard/package.json
Outdated
@@ -16,10 +16,13 @@ | |||
"@angular/core": "^4.2.2", | |||
"@angular/forms": "^4.2.2", | |||
"@angular/http": "^4.2.2", | |||
"@angular/material": "^2.0.0-beta.6", |
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.
If we make this point to the build snapshot it could help give us a heads-up if something goes wrong
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.
Yep. I agree
} | ||
|
||
/** Interface that describes the payload results from the Firebase database. */ | ||
interface PayloadResult { |
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.
Move this to a file like data-definitions.ts
?
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.
Thought about it but wanted to wait until we have more other components. I'm changing it anyway now.
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.
LGTM
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
EDIT: Maybe this is rather a
chore
instead of abuild change
. But should be fine though..