Deal with no flags passed to Server#destroy

[ekohl]

In 085a7c8 it became possible to pass flags to Server#destroy but if the caller didn't pass flags in opts it failed because it was essentially nil.zero? which isn't defined.

I wasn't sure if I should drop the defaults for flags in options, but decided to keep it in to "document" the API.

[adamruzicka]

Yes, this could work, although to me it doesn't express the intent as clearly as it could. What would you say to something like this as a counter proposal?

def destroy(options = {})
  defaults = {:destroy_volumes => false, :flags => 0}
  options = defaults.merge(options)
  # ...
end

I know this is a matter of personal taste so I won't insist on it.

[ekohl]

Good question. What I'd actually prefer is def destroy(destroy_volumes: false, flags: 0, **kwargs) but that's probably breaking the API and have issues with Ruby < 2.7.

[adamruzicka]

Yes, that would be even better. Do we have to keep compatibility with eol rubies? It is breaking the api, but we will have to do that eventually anyway

[ekohl]

I don't know to be honest. I also don't know if we need to remain compatible with some generic fog interface for destroy(). If we don't, kwargs could actually be dropped.

[ekohl]

For now I'm going to merge this. In Foreman we now pin to >= 0.9.0 (https://github.com/theforeman/foreman/blob/339fe635ffa37d52e7174be401e9b54af20c5681/bundler.d/libvirt.rb#L2) and that means that a new release on Debian would break if we change the API. So this isn't a long term solution, but it makes the code safer.