Update params to account for oracle linux.

[drfeelngood]

Check $::osfamily when setting nx_daemon_user. See issues #176 and #177

[3flex]

This will break Gentoo users using older Facter (pre 1.7) releases, where osfamily will return "Linux".

Should either add Linux to the selector or refactor to be a case statement with a default that returns "nginx" as the user.

[jfryman]

This is great, but I'd rather run in parallel with a deprecation warning if $::osfamily is not set. It sucks, but there are still installs that do not have it.

[drfeelngood]

Maybe something like this?

if $::osfamily {
  # case osfamily and warn if == linux but set to nginx 
} else {
  # set nx_daemon_user based on $::operatingsystem
  # jam oraclelinux in regex like #177? 
}

[jfryman]

@drfeelngood yeah, that looks good. I'd also add a warning('Support for $::operatingsystem will end with the release of Facter 2.0') and should get anyone who needs a solution time to work on it before it gets ripped out.

[drfeelngood]

@jfryman Does this correspond to what you're envisioning?

[3flex]

Add "linux" to the strings to match to avoid a regression (so Gentoo users on Facter versions >= 1.6.2 and < 1.7 still work). In these versions on Gentoo osfamily = "Linux" so the if expression is true, but the selector won't match anything.

[drfeelngood]

Argh, I had it then second guessed myself. All is well now.

[3flex]

I'm confused about the warning - what's the user being warned about? If they upgrade Facter (to the current version, or 2.0 when it comes out) things will just keep on working if this is merged. If the warning is because this module now deprecates operatingsystem fact support shouldn't that be the warning instead? That way users know to upgrade Facter.

[drfeelngood]

I guess with the current state of this code the warning might not be necessary from what you said. I don't think I'm the person to say "it's going to be deprecated", so possibly leaving it out for now makes the most sense. What do you think?

[3flex]

Oh I'm not sure either, that's @jfryman's call. Just not a fan of warnings that don't indicate what action is needed.

Oh and side note, I'm glad you fixed that warning for lsbmajdistrelease in code. I'm working on rspec tests for this module and keep having to define that fact (after always having failures because I forgot to do so), so I hope this is merged!

[jfryman]

We can be more explicit in the warning, but the intent is that in a few releases of something, I'm yanking this out. I really hate to keep edge cases in the code, so I'd rather let the user know it'll be soon to update. But yes, the warning should be when $::osfamily is not defined and we fall-back to $::operatingsystem.

I'm open to suggestion on how we communicate this to the user, but this needs to be here if we include this edge case so we don't accrue too much debt.

[drfeelngood]

How about something similar to the message found here

warning('$::osfamily not defined.  Support for $::operatingsystem is deprecated')
warning("Please upgrade from factor ${::facterversion} to >= 1.7.2")

[drfeelngood]

I went ahead and squashed my commits to clean things up.

[3flex]

👍

[jfryman]

Love it! Thanks so much! ❤️