-
Notifications
You must be signed in to change notification settings - Fork 37
Conversation
@@ -227,6 +227,16 @@ | |||
} | |||
}, | |||
create: { | |||
// Creates a scatterplot with a connecting sparkline |
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.
I generally prefer clear code over code comments because inevitably someone will change the code without updating the codes. Any way we could update this js to make it clearer / mitigate need for code comments?
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.
I agree that the names need some refactoring. It feels quite a bit outside the scope of this PR. Why don't I just remove the comment for now, and create a naming issue for later refactoring?
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.
that works for me!
looks like jasmine test are running on travis!! woot woot sadly, they are failing |
@jessieay I got pulled into some EITI stuff, so I'll take a look at it today. Running |
Update: I passed the mock data directly to the previous_winners_spec.js variable (instead of referencing it from a helper file). The issues continued to exist while running So why am I getting this error (below)?
|
Closing this in favor of #659 |
For #628
Adds the
jasmine:ci
to travis.Updates jasmine tests to pass.