Skip to content
This repository has been archived by the owner on Apr 6, 2021. It is now read-only.

Add query arg for delay parameter on lines #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add query arg for delay parameter on lines #2

wants to merge 4 commits into from

Conversation

khwilson
Copy link
Contributor

@khwilson khwilson commented Jan 2, 2019

This is a small change that allows for a power user to specify the headway discrepancy they want to call "bad service" as a query argument. It moves (most of) the computation to the client side in the process.

Currently only does it for the lines because I wanted to see if @blahblahblah- wanted the general thrust done in some other manner.

To allow the client to mess with good and bad service thresholds,
move the status computation (or at least part of it) to the client
side. This commit does it for the lines.
@blahblahblah-
Copy link
Owner

Hey, I thought I would have time to review this, but with getting back from the holidays and I'm leaving for vacation, I won't be able to get back to this until afterwards, which unfortunately will be a month from now. Sorry for the delay!

@khwilson
Copy link
Contributor Author

khwilson commented Jan 7, 2019

Hah, no problem. I'm using it on a personal server to grab historical delay times for now. Planning on publishing a little post, hopefully when you get back in town. :)

Copy link
Owner

@blahblahblah- blahblahblah- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thanks for doing this. Apologies again for the delay, I just returned from my vacation yesterday. Just a few nitpick comments regarding syntax and whether or not some of the code should be included in this PR.

@@ -241,9 +249,43 @@ class LandingPage extends React.Component {
if (TEST_DATA) {
this.setState({ trains: sampleData.routes, lines: sampleData.lines, loading: false });
} else {
let params = qs.parse(location.search);
let use_median = params.median && params.median === 'true';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tiny nitpick, let's keep variable names camelCase.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, don't think we need the first half of the evaluation if we are evaluating with ===?

import * as Cookies from 'es-cookie';

const API_URL = '/api/info';
const TEST_DATA = false;

function getQueryStringValue (key) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this if we already import query-string?

@@ -23,16 +23,60 @@ def delay

def max_actual_headway
@max_actual_headway if @max_actual_headway
compute_actual_headways
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revert changes of this file until the feature is implemented for routes as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants