This repository has been archived by the owner on Mar 4, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 149
feat(#64) added multiple aggregate levels #66
Merged
jasonwilson
merged 21 commits into
FormidableLabs:master
from
mscottx88:features/graph-units-of-time
Aug 8, 2017
Merged
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
f5666af
feat(#64) added multiple aggregate levels
22bc27a
fix(#64) remove vscode file first, then gitignore it
mscottx88 bdd3adf
fix(#64) gitignore vs code
mscottx88 e74e992
test(#64) added unit tests for MetricsProvider
64e7805
test(#64) removed gulp, removed duplicate setup
mscottx88 89a6fa2
test(#64) removed unnecessary done()
mscottx88 07871d1
fix(#64) test use sandbox stub
mscottx88 f3b8f2c
fix(#64) cross-platform fix to coverage
mscottx88 6dce469
chore(#64) simplified aggregation
mscottx88 7b373a1
chore(#64) converted to inline aggregation
mscottx88 6dd1f3b
test(#64) 100% code coverage MetricsProvider
mscottx88 939ba75
docs(#64) bumped version and added changelog entry
mscottx88 403853a
fix(#64) corrections for code review
mscottx88 b7eec6c
docs(#64) added notes to Changelog
mscottx88 ae31391
fix(#64) no more forced layouts
mscottx88 1b61876
fix(#64) improved time labels
mscottx88 d749914
fix(#64) getTimeIndexLabel is not a member function
mscottx88 07ddccb
fix(#64) lodash has padStart
mscottx88 dbd091e
fix(#64) memory guage view frozen
mscottx88 b3f7319
docs(#64) separate MD for aggregation
20c542f
docs(#64) correct PR ref
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
/node_modules | ||
npm-debug.log | ||
coverage | ||
.vscode |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
"use strict"; | ||
|
||
var gulp = require("gulp"); | ||
var mocha = require("gulp-mocha"); | ||
|
||
gulp.task("test-debug", function () { | ||
gulp.src(["test/**/*.spec.js"]) | ||
.pipe(mocha()); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
"use strict"; | ||
|
||
// these define the time levels that data will be aggregated into | ||
// each number is in milliseconds | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: Do the empty newline |
||
// since these are used as object keys, they are strings | ||
|
||
// For example, 5000 = 5s and means that one of the aggregate levels | ||
// will be at 5s increments of time | ||
|
||
// 300000 = 30s and the corresponding zoom will show 30s aggregates | ||
var AGGREGATE_TIME_LEVELS = [ | ||
"5000", | ||
"10000", | ||
"15000", | ||
"30000", | ||
"60000", | ||
"300000", | ||
"600000", | ||
"900000", | ||
"1800000", | ||
"3600000" | ||
]; | ||
|
||
// this array object is used to reduce ms to its highest human-readable form | ||
// see lib/providers/metrics-provider.js::getTimeIndexLabel | ||
var TIME_SCALES = [ | ||
{ | ||
units: "ms", | ||
divisor: 1 | ||
}, { | ||
units: "s", | ||
divisor: 1000 | ||
}, { | ||
units: "m", | ||
divisor: 60 | ||
}, { | ||
units: "h", | ||
divisor: 60 | ||
}, { | ||
units: "d", | ||
divisor: 24 | ||
}, { | ||
units: "y", | ||
divisor: 365.24 | ||
} | ||
]; | ||
|
||
module.exports = { | ||
AGGREGATE_TIME_LEVELS: AGGREGATE_TIME_LEVELS, | ||
TIME_SCALES: TIME_SCALES | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,12 @@ Dashboard.prototype._configureKeys = function () { | |
this.screen.key(["q", "C-c"], function () { | ||
process.exit(0); // eslint-disable-line no-process-exit | ||
}); | ||
|
||
this.screen.key(["up", "down"], _.throttle(function (ch, key) { | ||
var zoom = key.name === "down" ? -1 : 1; | ||
this.screen.emit("zoomGraphs", zoom); | ||
this._showLayout(this.currentLayout, true); | ||
}.bind(this), THROTTLE_TIMEOUT)); | ||
}; | ||
|
||
Dashboard.prototype.onEvent = function (event) { | ||
|
@@ -94,8 +100,8 @@ var VIEW_MAP = { | |
eventLoop: EventLoopView | ||
}; | ||
|
||
Dashboard.prototype._showLayout = function (id) { | ||
if (this.currentLayout === id) { | ||
Dashboard.prototype._showLayout = function (id, forced) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there another way to update the aggregate view without having to force a layout refresh? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sure there is. I will work on it! |
||
if (this.currentLayout === id && !forced) { | ||
return; | ||
} | ||
_.each(this.views, function (view) { | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Small nit here -- at least as a broad Formidable convention (and same for most of our clients), npm scripts have "won".
Gulp, while a great tool, has persistent problems of plugins being behind the "real" upstream projects and adding complexity / abstraction where it's not needed. Here, we just want to execute mocha. I vote we stick with mocha +
package.json:scripts
until we hit a use case we truly can't support.Side note -- if we need concurrent execution of things, we've got great tools for that that work with
package.json:scripts
.@jasonwilson -- Thoughts on ^^^ ?
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.
Can npm run scripts allow you to debug your spec files? If so, that is great. Otherwise, the only purpose for the gulp is for debugging unit tests and I can keep it out of the main repo.
Although, excluding it would make portability an issue across dev machines.
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.
First, you can always use
mocha
directly. Second npm scripts allow extra commands to punch to the command via--
. So, for example,works just fine.
Also, we write our npm package scripts to be the portable subset of shell commands that is universally (mac, win, linux) compatible.
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.
By dev machine, I really meant for those folks that might use VS code as their debugger, these tagalong files are helpful to include
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'm not familiar with VS code's debugging, but I'd be surprised if generic gulp was supported and raw mocha wasn't. In any case, let's keep the main repo gulp-free to save on dependencies we don't strictly need 😉
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.
Sounds good. Also sounds like I might have another interested party in learning debugging and using VS code?
I think you'll find it to be a really amazing editor.
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.
Since it is an Electron app, it is built on Node too, so this editor is perfectly adapted to Node development.
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.
Out of curiosity, since you're not using VS code, what editor are you using?
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 believe I have the solution I was looking for to remove the gulp and still get the debug ability.
https://gist.github.com/paambaati/54d33e409b4f7cf059cc
Up to date even!
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 use many, currently vs code. What I meant is that I don't use the integrated debugger -- typically when I need a debugger I just do the normal node flags and open up chrome.
Nice to see you found what you need and your preferred debugging solution is still available to you!