[COOK-3418] Virtual Domain Support PR

[bradenwright]

http://tickets.opscode.com/browse/COOK-3418

I resolved the merge conflicts on #15 and would like to get it merged.

Thanks to jdamick for his work

[jesseadams]

@bradenwright - Please see the comments in the ticket: https://tickets.opscode.com/browse/COOK-3418

[bradenwright]

Sorry I thought I had replied on the ticket.

I did see the comments and I would still like to get this merged in. I started working on the refactor but didn't have time to finish.... I should have time this weekend, if not earlier

[allaire]

👍 Currently I duplicated most of the work in this commit in my own fork's branch. @bradenwright tell me if you're too busy for the refactor, I could do it 🍺

[allaire]

I think it would be better to separate this condition, thoughts?

<% if !node['postfix']['virtual_domains'].empty? -%>
virtual_alias_domains = <%= node['postfix']['virtual_domains'].join(' ') %>
<% end -%>

<% if !node['postfix']['virtual_aliases'].empty? -%>
virtual_alias_maps = hash:/etc/postfix/virtual
<% end -%>

[bradenwright]

I completely agree that separating them would be better and will do so!

I've got time tonight and think I should be able to get it finished and committed up but if there is anything in your you prefer in your code, etc I'm open (or if you've already refactored I'd check out your branch and double check it works for me). Anyways let me know what you think.

[allaire]

@bradenwright Go ahead! 🍺

[bradenwright]

Alright so should be all set... I did rename/change the attributes to follow along with the new 3.0.1 format (so I didn't have to change the main.cf template).

[sethvargo]

Can we refactor this to something like:

default['postfix']['virtual']['virtual_aliases'] = {} 
unless node['postfix']['virtual']['virtual_aliases'].empty?
  default['postfix']['main']['virtual_alias_domains'] = "" 
  default['postfix']['main']['virtual_alias_maps']    = 'hash:/etc/postfix/virtual' 
end

[sethvargo]

@bradenwright there are some inline comments; could you please take a look? Don't forget to mark the JIRA ticket as "Fix Provided" after you're done.

[1ed]

👍

[bradenwright]

Thanks Seth! Think its ready to go... sorry there was so much to clean up for such little code.

[allaire]

@sethvargo We need that :)

[bradenwright]

looks like it got merged into master. Thanks!

(I assume I'm supposed to close this since its merge into master and still open if not let me know and I'll reopen it)

[bradenwright]

I really sorry I got confused and must have been on my github... this is not merged yet

[1ed]

@bradenwright could you please mark the ticket as "Fix Provided"

[allaire]

Any updates on this? I would gladly ditch my fork of this cookbook in favor of this PR :)

[bradenwright]

I made the changes that @sethvargo suggested so I think its just waiting on opscode to merge it in at this point.

It should be usable (but I get if want it to be merged into master before using it).

[allaire]

@bradenwright You should rebase/squash your 6 commits into 1, I'm sure @sethvargo will ask for it :)

[bradenwright]

Cool that makes total sense regardless I'll take care of that. I haven't squashed before but I've rebased, I'm sure its not very hard. I should have time to do it tonight or tomorrow (for sure this weekend).

[allaire]

@bradenwright git rebase -i HEAD~6 (where 6 is the number of commits) then git push --force

[bradenwright]

I got it all put into 1 commit but....

either I executed the rebase wrong or I couldn't figure out a great way to do it since there were other commits in between my 6 commits. I decided to grab the latest from postfix and cherry-pick what I needed and merge it onto latest master from postfix (since its only 4 files), then push force. But ultimately I accomplished what you suggested which is getting all my code into 1 commit.

Thanks again for the suggestion... I'll definitely start to use squashing commits in my everyday workflow ;)

[someara]

merged in d6bec0d

[bradenwright]

@someara FYI just wanted to let you know that looks like this essentially got added 2x #55 is doing the same thing and got merged as well. (Look at recipes virtual and virtual_aliases)

#55 looked to have more logic in the attributes so probably makes sense to back out this change and leave #55 (either way 1 of them should be backed out).

[someara]

reverted. thanks!
-s