search

saltstack-formulas / haproxy-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
28 stars 122 forks source link

Use k,v in template #34

Closed morsik closed 8 years ago

morsik commented 8 years ago

You had fantasy for using list in .iteritems()… :D

Not tested. Changed "on fly" when I looked into template file.

gravyboat commented 8 years ago

Could you please test it? This is a large modification.

morsik commented 8 years ago

Fixed one thing, looks like it works now.

iggy commented 8 years ago

You might as well replace iteritems with items while you are in there...

morsik commented 8 years ago

Is |dictsort filter ok? ;) Should be better than .items()/.iteritems(), see #16.

gravyboat commented 8 years ago

@morsik We now have a conflict of your usage of dictsort and the recently merged https://github.com/saltstack-formulas/haproxy-formula/pull/37 that @abednarik wrote. @abednarik do you have any concerns over using dictsort here? I don't have any, but I also don't use haproxy at all so I'm not sure if the sorting would be problematic for some users.

abednarik commented 8 years ago

Ok. Also I don't have any complain regarding dictsort as long as formula works good I'm just fine. We can drop this PR.

gravyboat commented 8 years ago

@abednarik Yours was already merged, @morsik needs to rebase, no worries!

morsik commented 8 years ago

Oh god…

gravyboat commented 8 years ago

@morsik What's wrong? Rebase should detect his updates versus your changes and there should be no issue, just use a merge strategy of yours instead of his.

morsik commented 8 years ago

I hate those conflicts... check now, and don't make conflicts when it's not needed next time, please? This rebase took me more time than changing that code btw at the first time…

gravyboat commented 8 years ago

@morsik I also don't like the conflicts but it happens, none of us are paid to work on this stuff as we're all volunteers, sometimes mistakes happen. Thanks for fixing it up.

abednarik commented 8 years ago

Sorry for any inconvenience and thanks both.

Cheers El nov. 24, 2015 5:55 PM, "Forrest" notifications@github.com escribió:

Merged #34 https://github.com/saltstack-formulas/haproxy-formula/pull/34 .

— Reply to this email directly or view it on GitHub https://github.com/saltstack-formulas/haproxy-formula/pull/34#event-473781515 .

morsik commented 8 years ago

Thanks, unfortunaletly GitHub doesn't have option to do Rebase so I had to it manually (and learn how to do it :P)