New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add de馃悰 view for timings for Git operations (including queue times) #508

Merged
merged 20 commits into from Feb 14, 2017

Conversation

Projects
None yet
3 participants
@BinaryMuse
Member

BinaryMuse commented Feb 8, 2017

The goal is to implement a nice UI around this, but for now I'll just record some data and throw it in a table or something.

Resolves #507

@BinaryMuse BinaryMuse self-assigned this Feb 8, 2017

@BinaryMuse BinaryMuse changed the title from Add timings for Git operations (including queue times) to Add de馃悰 view for timings for Git operations (including queue times) Feb 8, 2017

simurai added some commits Feb 8, 2017

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Feb 8, 2017

Member

Replace with 馃懢

@BinaryMuse Feel free to revert 鈽濓笍 , but thought it's kinda funny.

image

Member

simurai commented Feb 8, 2017

Replace with 馃懢

@BinaryMuse Feel free to revert 鈽濓笍 , but thought it's kinda funny.

image

@@ -1,22 +1,28 @@
@import "variables";

This comment has been minimized.

@simurai

simurai Feb 8, 2017

Member

@BinaryMuse Just FYI: If you want to use the theme variables you'll have to @import "variables"; which imports the ui-variables.less. I think otherwise it will use the fallback variables from core, which could be off.

@simurai

simurai Feb 8, 2017

Member

@BinaryMuse Just FYI: If you want to use the theme variables you'll have to @import "variables"; which imports the ui-variables.less. I think otherwise it will use the fallback variables from core, which could be off.

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Feb 8, 2017

Member

Any way to strip those "in + path". Or is that vital information?

screen shot 2017-02-08 at 7 31 00 pm

Member

simurai commented Feb 8, 2017

Any way to strip those "in + path". Or is that vital information?

screen shot 2017-02-08 at 7 31 00 pm

@BinaryMuse

This comment has been minimized.

Show comment
Hide comment
@BinaryMuse

BinaryMuse Feb 8, 2017

Member

@simurai Thanks :)

This will be a waterfall view soon enough

Member

BinaryMuse commented Feb 8, 2017

@simurai Thanks :)

This will be a waterfall view soon enough

@smashwilson

This comment has been minimized.

Show comment
Hide comment
@smashwilson

smashwilson Feb 8, 2017

Member

馃拑 niiice.

How would you feel about me refactoring this into timecop as a general tool at some point? You know, in my copious free time. Evolving the timecop package into a general-purpose set of performance monitoring and metrics visualization tools seems like a natural fit to me.

Member

smashwilson commented Feb 8, 2017

馃拑 niiice.

How would you feel about me refactoring this into timecop as a general tool at some point? You know, in my copious free time. Evolving the timecop package into a general-purpose set of performance monitoring and metrics visualization tools seems like a natural fit to me.

@BinaryMuse

This comment has been minimized.

Show comment
Hide comment
@BinaryMuse

BinaryMuse Feb 9, 2017

Member

How would you feel about me refactoring this into timecop as a general tool at some point?

Maybe! Especially if we find it brings values to a lot of users without bloating it too much. That's a ways off though; for now I'd prefer to iterate on the functionality here and see how it shakes out.

Member

BinaryMuse commented Feb 9, 2017

How would you feel about me refactoring this into timecop as a general tool at some point?

Maybe! Especially if we find it brings values to a lot of users without bloating it too much. That's a ways off though; for now I'd prefer to iterate on the functionality here and see how it shakes out.

@BinaryMuse

This comment has been minimized.

Show comment
Hide comment
@BinaryMuse

BinaryMuse Feb 14, 2017

Member

Okay, the timings table has been converted into a waterfall view. This makes it easier to see how operations relate to each other.

For example, in this case, the highlighted command (git rev-parse --abbrev-ref HEAD) completed in just 15ms, but it was queued for more than 300ms:

fullscreen_2_13_17__5_04_pm

There is also an import/export feature to allow capturing and sharing profiles:

git-timings

@simurai I did the best I could with the styling, but there's still an issue where the individual rows don't extend all the way to the right side of the container. You can see this in the 2n+1 row background colors:

github_package_timings_view

Another designer and I looked at it for a while but weren't able to come up with anything. I'm sure my abundant usage of position: absolute is to blame.

Member

BinaryMuse commented Feb 14, 2017

Okay, the timings table has been converted into a waterfall view. This makes it easier to see how operations relate to each other.

For example, in this case, the highlighted command (git rev-parse --abbrev-ref HEAD) completed in just 15ms, but it was queued for more than 300ms:

fullscreen_2_13_17__5_04_pm

There is also an import/export feature to allow capturing and sharing profiles:

git-timings

@simurai I did the best I could with the styling, but there's still an issue where the individual rows don't extend all the way to the right side of the container. You can see this in the 2n+1 row background colors:

github_package_timings_view

Another designer and I looked at it for a while but weren't able to come up with anything. I'm sure my abundant usage of position: absolute is to blame.

BinaryMuse and others added some commits Feb 14, 2017

@BinaryMuse BinaryMuse merged commit 2922a70 into master Feb 14, 2017

3 of 5 checks passed

schema/pr GraphQL Schema out of date
Details
schema/push GraphQL Schema out of date
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@BinaryMuse BinaryMuse deleted the mkt-de馃悰-ui branch Feb 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment