search

saltstack-formulas / postfix-formula

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

Restructured to Add Support for FreeBSD #59

Closed 0xf10e closed 6 years ago

0xf10e commented 7 years ago

Hi @EvaSDK, @davidkarlsen, @imran1008,

I had to adjust quite a bit to add support for FreeBSD so the diff became rather big so I'm pinging you main contributors here.

Would be great if someone can check if those changes won't break existing setups. The actual changes to managed files should be rather limited like some options in main.cf moving around because of the for-loop I added in 8eed2547735865.

The FreeBSD jail I've made this changes for isn't in production yet as I have to add things like spamfilters first but it's happily sending cron mails already.

Regards, Florian

PS: @aboe76 looks like I can't request reviews from these guys??

0xf10e commented 7 years ago

@EvaSDK The BSDs separate the base system (which provides a complete OS) from third party applications. Things installed from ports or packages go to /usr/local.

0xf10e commented 7 years ago

So I set up a Linux VM to test while I rework those commits (again) and guess what: The current version (as of 7fc82e0bd2cebb63443c71c6a8c3381b2555c20b) is broken on Debian 9.

0xf10e commented 7 years ago

Fun fact: The version from my branch at least applies cleanly (no functional tests).

0xf10e commented 7 years ago

Sorry, not gonna start over again to first fix things for linux before I can even start generalizing everything to then add support for FreeBSD when all of those things are already done in my current version.

EvaSDK commented 7 years ago

Sorry, not gonna start over again to first fix things for linux before I can even start generalizing everything to then add support for FreeBSD when all of those things are already done in my current version.

The comment I made about commit content is purely about good practices for your future commits, I do not expect you to redo them now.

As for the formula being supposedly broken currently, I would object that my minions run current master just fine :smile: .

0xf10e commented 7 years ago

Yeah, there might have been caffeine withdrawal (I didn't expect o0) involved, my mood was pretty bad all day long ^^"

I don't know if I'll have the time to replace the macro with my vacation being over and no need for this at work.

And I've only ran a quick check on debian 9. Which distro do you have on your minions?

EvaSDK commented 7 years ago

I have Ubuntu 16.04 LTS, Debian 8 and 9 and CentOS 6 minions.

0xf10e commented 7 years ago

nice, you can provide all the guinea pigs I don't have, @EvaSDK :D

0xf10e commented 7 years ago

I think I'll try to restructure the defaults/maps stuff to use the defaults module then.

alxwr commented 6 years ago

@aboe76 @EvaSDK @0xf10e From my perspective the only thing missing is the default/maps stuff. I added that in #62.

If you agree with my changes there, I'd be happy to merge this PR and then #62.

aboe76 commented 6 years ago

@alxwr I'm ok with it, if @EvaSDK and @0xf10e don't see a probleem please proceed.

0xf10e commented 6 years ago

Sorry, just finished the setup I've added the FreeBSD support for before the holidays and didn't get to the cleanup part yet…

Got "review and make more PRs" in my new year's resolutions… ;)