search

theory / svn-notify

Subversion activity notification
http://search.cpan.org/dist/SVN-Notify/
18 stars 18 forks source link

Merge inline css patch #12

Closed adamcrews closed 3 years ago

adamcrews commented 9 years ago

I have been able to rebase the patches from @fgiust to get the inline css working. This makes the html emails look good in gmail.

It seems to work however I'm not able to get the tests passing due to some UTF8 Errors. I'm using this patched version without issue so far. If someone can help fix up the tests, I think this is a great contribution to the module.

theory commented 9 years ago

I don't understand why you have removed the original method of putting the CSS in a message. Why not add an option to do inline instead of a style block, but keep the style block as the default?

fgiust commented 9 years ago

On 2 April 2015 at 23:43, David E. Wheeler notifications@github.com wrote:

Why not add an option to do inline instead of a style block, but keep the style block as the default?

The style block is not read by web-based email clients, nowadays it makes no much sense formatting a mail that way IMHO.

fabrizio

— Reply to this email directly or view it on GitHub https://github.com/theory/svn-notify/pull/12#issuecomment-89053412.

theory commented 9 years ago

That's no reason to remove functionality that was working perfectly well in lots of other mail clients.

I appreciate the work here, but I don't think it's a good idea to remove something that was there before.

kbower commented 8 years ago

What's the probability this will ever make it in?

theory commented 8 years ago

@kbower It would need to be revised to turn inline css into an option.