search

premailer / css_parser

Ruby CSS Parser
Other
279 stars 110 forks source link

Handling nil order in split_declaration #73

Closed cbillen closed 8 years ago

cbillen commented 8 years ago

This will fix issues reported on premailer https://github.com/premailer/premailer/issues/290

grosser commented 8 years ago

can you add a test too so nobody breaks this after it's merged ? :)

cbillen commented 8 years ago

I’d love to i’m just not sure how to create a css_parser specific test for the condition in the first place

[image: WarmlyYours] https://www.warmlyyours.com [image: Facebook] http://www.facebook.com/warmlyyours [image: Twitter] http://www.twitter.com/warmlyyours [image: LinkedIn] http://www.linkedin.com/company/warmlyyours Christian Billen Director of IT P: (800) 875-5285 x 800 F: (800) 408-1100 Tell us how we're doing https://www.warmlyyours.com/survey/customer-satisfaction/quick-survey-25 and receive $25 off your next order! http://www.houzz.com/pro/warmlyyours1 http://www.houzz.com/pro/warmlyyours1 [image: SEN 2015 Vendor of the Year]

On Sat, Jul 23, 2016 at 12:21 PM, Michael Grosser notifications@github.com wrote:

can you add a test too so nobody breaks this after it's merged ? :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/premailer/css_parser/pull/73#issuecomment-234729691, or mute the thread https://github.com/notifications/unsubscribe-auth/AABicB3H65HbZgVMlljAEEkTemlxj85Dks5qYk2agaJpZM4JTZAi .

akzhan commented 8 years ago

@grosser I have merged it and updated to check dest nil case. But @cbillen can add tets by another PRq.

grosser commented 8 years ago

I don't like "merge and add test later" because the adding test part usually never happens ;) ... not sure how urgent this fix was / why using a branch was not ok ... but not important ...

akzhan commented 8 years ago

You also true. remember me about at friday, I'll see too.

2016-07-25 17:58 GMT+03:00 Michael Grosser notifications@github.com:

I don't like "merge and add test later" because the adding test part usually never happens ;) ... not sure how urgent this fix was / why using a branch was not ok ... but not important ...

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/premailer/css_parser/pull/73#issuecomment-234978569, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG3hBd9Zw9HpBxTf5bG2P1CXQHjGTSxks5qZM8UgaJpZM4JTZAi .