Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust Left Margin and time axis to fit #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Mar 27, 2019

  • The left margin is adjusted using an estimation based on the width of the largest label (target name) – Closes Target names in graph are truncated #8
  • The time axis now uses the .timeDomainMode("fit") (instead of .timeDomainMode("fixed") with a fixed 10:00 max value) – Closes X Axis max value too big #9
  • Theoretical Minimum is now rounded to two digits (to avoid the .33333333333 kind of values)

@AliSoftware AliSoftware force-pushed the feature/axis-adjust branch from 55b3874 to 541588c Compare March 27, 2019 19:20
@AliSoftware
Copy link
Contributor Author

Side note: I'm a bit confused as the .gemspec in this repo mentions a version 0.1.0, but on RubyGems the latest published version is 0.1.1.

Did you forget to commit the version bump from the .gemspec when you did push the 0.1.1 to rubygems?
In any case, I guess for the next release you'll have to remember to bump the xcode-build-times.gemspec to 0.1.2 (and not 0.1.1 again) 😉

@PaulTaykalo
Copy link
Owner

Right.
The main idea was to be able to compare speed of two different runs.
It will be really hard to do with scaled results
If we're about to play with scaling, I would rather go with some parameter. If possible.

@khanlou
Copy link

khanlou commented Apr 19, 2019

But that's what this PR does. Specifically, the line .timeDomainMode("fit") makes it automatically adjust the graph to be full width. I tried it out locally on my generated javascript files and it works great.

@PaulTaykalo
Copy link
Owner

My point is that with this size to fit option, it is hard to visually compare two runs.
So I suggest to have both "fit" and "fixed' width options, and not having simple switch to one of the two options. Both solutions are needed.

  • fixed - for comparing different builds (i.e. if you're about to speed up build time)
  • fit - to compare different parts of the build

@PaulTaykalo
Copy link
Owner

For example. having additional option passed in here
https://github.com/PaulTaykalo/xcode-build-times-rendering/blob/master/lib/xcode-build-times.rb#L111

for switching between fixed and fit variants

@PaulTaykalo
Copy link
Owner

JFYI: I really grateful for this PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

X Axis max value too big Target names in graph are truncated
3 participants