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

Canvas prebuilt #160

Merged
merged 12 commits into from
Oct 20, 2018
Merged

Canvas prebuilt #160

merged 12 commits into from
Oct 20, 2018

Conversation

Munter
Copy link
Owner

@Munter Munter commented Oct 18, 2018

Switched to canvas-prebuilt and did a lot of maintenance.

@papandreou This might be a way to remove the cairo dependency in assetgraph-sprite as well

@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 83.582% when pulling 21b52cd on canvas-prebuilt into 36bed61 on master.

@coveralls
Copy link

coveralls commented Oct 18, 2018

Coverage Status

Coverage increased (+2.2%) to 83.582% when pulling 1c57b3f on canvas-prebuilt into 7e14f8a on master.

Copy link
Collaborator

@papandreou papandreou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! 🚀

package.json Outdated
"mocha-chrome": "1.1.0",
"nyc": "13.1.0",
"uglify-js": "3.4.9",
"unexpected": "10.39.1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you switching to exact version numbers?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Not intentionally

@@ -33,6 +33,7 @@ describe('Histogram of gradient.png', function () {

it('should be greyscale', function (done) {
histogram(path, function (error, result) {
expect(error, 'to be', null);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: 'to be falsy'

@Munter
Copy link
Owner Author

Munter commented Oct 20, 2018

Interesting. I added the jpeg and gif tests to ensure that we could also drop those dependencies on libjpeg8-dev and libgif-dev completely. At least I recall those only being needed to support canvas operations.

The added tests, which show up green on this PR, fail on my machine :/

  Histogram of cablecar.gif
dyld: lazy symbol binding failed: Symbol not found: __ZN2v87Isolate19CheckMemoryPressureEv
  Referenced from: /Users/munter/git/node-histogram/node_modules/canvas-prebuilt/canvas/build/Release/canvas.node
  Expected in: flat namespace

dyld: Symbol not found: __ZN2v87Isolate19CheckMemoryPressureEv
  Referenced from: /Users/munter/git/node-histogram/node_modules/canvas-prebuilt/canvas/build/Release/canvas.node
  Expected in: flat namespace

sh: line 1: 26747 Trace/BPT trap: 5       mocha
npm ERR! Test failed.  See above for more details.

@Munter
Copy link
Owner Author

Munter commented Oct 20, 2018

Updating from node 8.9.x to 8.12, clearing node modules and reinstalling fixed that problem

@Munter
Copy link
Owner Author

Munter commented Oct 20, 2018

I'm sufficiently satisfied that there are no regressions. I'll merge it and do a patch release. Then have a look at the corresponding change in assetgraph-sprite

@Munter Munter merged commit af1c598 into master Oct 20, 2018
@Munter Munter deleted the canvas-prebuilt branch October 20, 2018 08:54
@Munter
Copy link
Owner Author

Munter commented Oct 20, 2018

Released in v3.0.1

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.

3 participants