-
Notifications
You must be signed in to change notification settings - Fork 13
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
various fixes for 0.6 #12
Conversation
src/boundingbox.jl
Outdated
@@ -26,11 +26,11 @@ BoundingBox(width, height, depth) = BoundingBox(0mm, 0mm, 0mm, width, height, de | |||
|
|||
isabsolute{P1, P2}(b::BoundingBox{P1, P2}) = isabsolute(b.x0) && isabsolute(b.a) | |||
|
|||
typealias AbsoluteBox{N} BoundingBox{NTuple{N, Length{:mm, Float64}}, | |||
const AbsoluteBox{N} = BoundingBox{NTuple{N, Length{:mm, Float64}}, |
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.
This needs a @compat
to work on 0.5 right?
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.
Yeah, working on it rn
src/point.jl
Outdated
typealias Vec{N} NTuple{N, Measure} | ||
typealias Vec2 Vec{2} | ||
typealias Vec3 Vec{3} | ||
const Vec{N} = NTuple{N, Measure} |
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.
Same here
src/point.jl
Outdated
typealias AbsoluteVec{N} NTuple{N, Length{:mm, Float64}} | ||
typealias AbsoluteVec2 AbsoluteVec{2} | ||
typealias AbsoluteVec3 AbsoluteVec{3} | ||
const AbsoluteVec{N} = NTuple{N, Length{:mm, Float64}} |
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.
And here
src/point.jl
Outdated
typealias Vec{N} NTuple{N, Measure} | ||
typealias Vec2 Vec{2} | ||
typealias Vec3 Vec{3} | ||
const Vec{N} = NTuple{N, Measure} |
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 think an @compat
is needed when both sides are parameterized
Codecov Report
@@ Coverage Diff @@
## master #12 +/- ##
======================================
Coverage 33.8% 33.8%
======================================
Files 6 6
Lines 71 71
======================================
Hits 24 24
Misses 47 47
Continue to review full report at Codecov.
|
Thank you all for the quick feedback, it looks like everything is passing on both 0.5 and 0.6. I'll wait for the mac os builds to pass before merging. |
src/boundingbox.jl
Outdated
NTuple{N, Length{:mm, Float64}}} | ||
typealias Absolute2DBox BoundingBox{Tuple{AbsoluteLength, AbsoluteLength}, | ||
@compat const Absolute2DBox = BoundingBox{Tuple{AbsoluteLength, AbsoluteLength}, |
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.
might not be important, but i don't think @compat
is needed here since the left hand side is not parameterized
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.
Let me try without it, I guess we should limit the number of calls to @compat
to be safe
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.
if it works without, then there are a few other places it can be removed as well
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.
Yeah, it seems to be working. I'll remove it for all nonparamterized typealiasing calls
All green, LGTM |
Needed for GiovineItalia/Gadfly.jl#930