mozilla / telemetry-dashboard

Web-frontend for aggregated telemetry data
telemetry.mozilla.org
Other
89 stars 113 forks source link

Implemented option to Hide the "spill" bucket from categorical histograms behind an advanced setting #578

Closed ab0092 closed 6 years ago

ab0092 commented 6 years ago

@chutten i have done the required changes, but we have a problem if we use the spread operator while creating a new copy of the histogramList because each Histogram has a map function attached to its prototype property that is why i have used Object.Create using the Histogram's prototype or else the code will not work as expected. Please let me know if we can come up with a workaround or we are going to keep that code section unchanged. Thanks..

chutten commented 6 years ago

Ah yes, and that would be used during the conversion to data arrays for metrics graphics to display. That makes sense, and makes a shallow copy not work. We're fine without changing that section.

ab0092 commented 6 years ago

I made a mistake with my editor and ended up getting the whole dist.js formatted. Any idea how i can roll it back. I have already commited that.

chutten commented 6 years ago

You can try git revert HEAD which will make a commit that undoes everything in the HEAD commit. That'll get your branch back to the state where it was before your most recent commit, but it'll leave your commits on the branch so you can look at them (with git show HEAD^ for instance).

If you don't want to leave the commits on the branch you can git reset --hard HEAD^ which will reset your branch back to the state it was before the most recent commit by deleting your most recent commit from history.

ab0092 commented 6 years ago

Thanks for the help. I rolled it back and committed the required changes.

ab0092 commented 6 years ago

I think these changes this should do it. :-)

ab0092 commented 6 years ago

Ya i some how i added the extra indent.

ab0092 commented 6 years ago

As for the code on line no 253 - 255 in dist.js the code already follows an indentation with the rest of the code. Do you want any changes there??

chutten commented 6 years ago

It isn't the indentation in this case. In this case you deleted a line that I think might reduce the readability of the code. (whitespace is helpful)

ab0092 commented 6 years ago

Is there anything else that i am missing here or are we good to merge this ???

chutten commented 6 years ago

That's it, that's all, your code is now merged!

ab0092 commented 6 years ago

Thanks.. And sorry for the slight delay in completing the formatting changes.. :-)