Skip to content
This repository was archived by the owner on Feb 18, 2019. It is now read-only.

Conversation

@nihal111
Copy link
Contributor

@nihal111 nihal111 commented May 19, 2016

This is in reference to #45. As of now I am just logging the Total number of results in the console everytime the "Load" button or the fillTable() function is called. Is this approach alright or should I create a function similar to resultsModel.getResults()? @martiansideofthemoon @jgraham


This change is Reviewable

@arpan98
Copy link
Contributor

arpan98 commented May 19, 2016

Very elegant!

@jgraham
Copy link
Member

jgraham commented May 19, 2016

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


angular_scripts.js, line 483 [r1] (raw file):

    }

    resultsModel.getResults($scope.filter, $scope.runs, minTestId, maxTestId, Infinity)

This seems like it will be rally bad for performance since we end up doing the entire query twice. It should be possible to count the results without actually returning them, which might be a little better (although I bet it is still slow).


angular_scripts.js, line 485 [r1] (raw file):

    resultsModel.getResults($scope.filter, $scope.runs, minTestId, maxTestId, Infinity)
      .then((results) => {
        console.log("Total="+results.length);

Logging this in the console isn't what's wanted; it should be displayed on the page somewhere.


Comments from Reviewable

@jgraham
Copy link
Member

jgraham commented May 19, 2016

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@martiansideofthemoon
Copy link
Contributor

Review status: all files reviewed at latest revision, 3 unresolved discussions.


angular_scripts.js, line 483 [r1] (raw file):

    }

    resultsModel.getResults($scope.filter, $scope.runs, minTestId, maxTestId, Infinity)

@jgraham @Nihal, I was thinking that it will be a good idea if we get the number of results for each STATUS, so that we can use this to plot a pie-chart for comparison. What do you think about this?


Comments from Reviewable

@nihal111
Copy link
Contributor Author

angular_scripts.js, line 485 [r1] (raw file):

Previously, jgraham wrote…

Logging this in the console isn't what's wanted; it should be displayed on the page somewhere.

I know. I just wanted to approve the approach.

Comments from Reviewable

@nihal111
Copy link
Contributor Author

Comments from Reviewable

@nihal111
Copy link
Contributor Author

angular_scripts.js, line 483 [r1] (raw file):

Previously, martiansideofthemoon (Kalpesh Krishna) wrote…

@jgraham @Nihal, I was thinking that it will be a good idea if we get the number of results for each STATUS, so that we can use this to plot a pie-chart for comparison. What do you think about this?

Number of results for each STATUS after applying a filter, or without it? If after, then I guess the above approach wouldn't really be bad for performance if we find a similar way to get numbers for all STATUS in just one call.

Comments from Reviewable

@martiansideofthemoon
Copy link
Contributor

Review status: all files reviewed at latest revision, 3 unresolved discussions.


angular_scripts.js, line 483 [r1] (raw file):

Previously, nihal111 (Nihal Singh) wrote…

Number of results for each STATUS after applying a filter, or without it? If after, then I guess the above approach wouldn't really be bad for performance if we find a similar way to get numbers for all STATUS in just one call.

I agree with you, especially because the query is not fully correct right now. What do you think @jgraham?

Comments from Reviewable

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.

4 participants