search

andrewrk / juice

Juice inlines CSS stylesheets into your HTML source.
MIT License
60 stars 13 forks source link

add option applyLinksToStyleTags #11

Closed jrit closed 10 years ago

jrit commented 10 years ago

Intended to resolve https://github.com/andrewrk/juice/issues/10

I want to see if this passes on Travis since I'm concerned about the path resolution on Linux. Also needs documentation, which I'll provide.

andrewrk commented 10 years ago

Looks like a release commit got into the branch somehow?

jrit commented 10 years ago

Looks like its pulled from your master, not sure why its showing up here though. There is definitely a problem with the path on Linux though so I'll need to fix that before anything gets merged.

jrit commented 10 years ago

I'm unsure if my tests are failing because there is actually something wrong, or it has to do with the change to pend from batch in the test runner causing some weirdness. I do think the outstanding issue with the linux paths is fixed in this though.

jrit commented 10 years ago

@andrewrk do you have any thoughts on the test failures in this?

andrewrk commented 10 years ago

Nope. I have not looked into it at all.

jrit commented 10 years ago

Passing, finally.

andrewrk commented 10 years ago

Nice. Taking a look

andrewrk commented 10 years ago

This looks good. Can you update the README documentation so that users can find out about this feature and how to use it?

jrit commented 10 years ago

@andrewrk I updated the readme and also updated the new option to default to true. I'm not sure if this is the best way to add in the feature or not. The behavior of the new option seems more in line with what should have been happening to begin with, so let me know if you have feelings one way or the other about how this should be handled and I can make any final adjustments.

jrit commented 10 years ago

@andrewrk just checking if you saw this was ready to go unless you think the options should be merged

andrewrk commented 10 years ago

Sorry for the delay. Would you like to be a maintainer of this module, @jrit ?

jrit commented 10 years ago

Sure, I'd be happy to be a maintainer. Thanks.

andrewrk commented 10 years ago

npm username?

jrit commented 10 years ago

also jrit https://www.npmjs.org/~jrit

andrewrk commented 10 years ago

Done, thanks for the help.

jannisg commented 10 years ago

Was this ever published to NPM?

Been trying to figure out why the applyLinksToStyleTags option didn't work in my workflow and upon digging into the juice2 lib/juice.js file I noticed all the changes of this merge missing from my fileset.

I have ran npm cache clean and reinstalled the module afterwards but still pulling in the old (without applyLinksToStyleTags option) code.

jrit commented 10 years ago

@jannisg it wasn't, thanks for pointing that out, it is up to date now