Closed
Bug 870210
Opened 11 years ago
Closed 11 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•11 years ago
|
||
Finished updating README, this is ready for review
Attachment #747407 -
Flags: review?(schranz.m)
Comment 2•11 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•11 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•11 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•11 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•11 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•11 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: 11 years ago
Resolution: --- → FIXED
Updated•11 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
•