Closed
Bug 870210
Opened 12 years ago
Closed 12 years ago
Extend node-hubble to allow discovery of image sizes
Categories
(Webmaker Graveyard :: General, enhancement)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: humph, Assigned: humph)
References
Details
Attachments
(1 file)
We would like to know when to use an image based on its size, when to crop it, etc. Hubble is a perfect candidate for this. I've written the code and tests, just need to fixup the readme, then I'll get this into review.
WIP here - https://github.com/mozilla/node-hubble/pull/13
Assignee | ||
Comment 1•12 years ago
|
||
Finished updating README, this is ready for review
Attachment #747407 -
Flags: review?(schranz.m)
Comment 2•12 years ago
|
||
Comment on attachment 747407 [details] [review]
https://github.com/mozilla/node-hubble/pull/13
The code looks very sane to me, but I'm getting test fails when running locally. Specifically:
1) should give proper sizes for JPEG files
2) should give proper sizes for GIF files
And assertion errors on status codes. This leaves me thinking I'm somehow missing a dependency for the tests? I'm unsure, but everything I needed installed successfully with npm.
Attachment #747407 -
Flags: review?(schranz.m)
Attachment #747407 -
Flags: review?(jon)
Attachment #747407 -
Flags: review-
Assignee | ||
Comment 3•12 years ago
|
||
Take a look at your node_modules/canvas/build/canvas.target.mk, and see if gyp found the right deps. It should have defined HAVE_JPEG and HAVE_GIF. Here's what I have after I build:
DEFS_Release := \
'-D_DARWIN_USE_64_BIT_INODE=1' \
'-D_LARGEFILE_SOURCE' \
'-D_FILE_OFFSET_BITS=64' \
'-DHAVE_FREETYPE' \
'-DHAVE_JPEG' \
'-DHAVE_GIF' \
'-DBUILDING_NODE_EXTENSION'
Flags: needinfo?(schranz.m)
Comment 4•12 years ago
|
||
Yeah, I'm missing JPEG and GIF from that list. Rather than poke around more I'll let :jbuck look at it. Code R+ from me.
Flags: needinfo?(schranz.m)
Assignee | ||
Comment 5•12 years ago
|
||
Comment on attachment 747407 [details] [review]
https://github.com/mozilla/node-hubble/pull/13
Kind sir, a moment of your time?
Comment 6•12 years ago
|
||
Comment on attachment 747407 [details] [review]
https://github.com/mozilla/node-hubble/pull/13
r+, although I'm not a huge fan of the node-canvas dependency. It'd be nice to have something pure JS... if only we had unlimited time!
Attachment #747407 -
Flags: review?(jon) → review+
Assignee | ||
Comment 7•12 years ago
|
||
https://github.com/mozilla/node-hubble/commit/e30151186941e841721312747b16bf4d4977e6d6
The node-canvas dep isn't a big deal for a server. Basically, once it's doable in JS, node-canvas will switch to it too, so we'll all be happy.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in
before you can comment on or make changes to this bug.
Description
•