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

perf(bp): add min/max numbers to bp report #1233

Closed
wants to merge 1 commit into from

Conversation

rkirov
Copy link
Contributor

@rkirov rkirov commented Jul 15, 2014

The benchmark fluctuates quite a bit (even with the last 20 window), so I find the minimum time taken useful.

@@ -177,7 +177,8 @@ bp.Report.getTimesPerAction = function(name) {
tpa[c] = {
recent: undefined,
history: [],
avg: {}
avg: {},
minmax: {}
Copy link
Contributor

Choose a reason for hiding this comment

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

What about

minmax {
  max: 0,
  min: double.MAX_FINITE,
}

to simplify the code ?

edit: or even min and max ad 2 fields ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Number.MAX_VALUE;

@vicb
Copy link
Contributor

vicb commented Jul 15, 2014

lgtm, 1 comment

@vicb
Copy link
Contributor

vicb commented Jul 15, 2014

well actually may be one other comment, should it be labeled "perf" or "chore/feat" ? ("perf" being reserved for changes that actually improve perf)

@rkirov
Copy link
Contributor Author

rkirov commented Jul 15, 2014

Thanks for the quick review vicb! Take another look.

Also, I am new to this dev flow. I used commit --amend and push -f, let me know if I am doing something wrong.

@rkirov rkirov added cla: yes and removed cla: no labels Jul 15, 2014
@vicb
Copy link
Contributor

vicb commented Jul 15, 2014

👍
commit --amend and push -f is the way to go, no problem.

@rkirov rkirov closed this in d7bfc47 Jul 23, 2014
rkirov added a commit that referenced this pull request Jul 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

3 participants