Fix sending notifications from the ssl_certificate resource

[karlsvec]

Currently, the "updated" status of the ssl_certificate resource is determined by
the very last internal file resource from the provider that gets processed.
However, we want the ssl_certificate resource to send a notification if any of
the internal file resources updates, not just the last one.

[karlsvec]

Travis CI complains that the file_create method has too many lines. 11 lines is too many? Really?

[zuazo]

Hi @karlsvec,

Thanks again for your contributions!

The Ruby Style Guide says:

Avoid methods longer than 10 LOC (lines of code). Ideally, most methods will be shorter than 5 LOC. Empty lines do not contribute to the relevant LOC.

Discussed in rubocop/rubocop#494:

The subject is not entirely without debate. Some studies indicate that long methods are actually less error-prone than short ones, but you have to go with your conviction some times and not blindly follow the numbers. Some very influential people advocate short methods. Robert C . Martin talks about it in his book Clean Code, Martin Fowler lists Long Method as a code smell in Refactoring, and Sandi Metz sets the limit at 5 in her rules.

I'm not against relaxing some rules when required, but I try to avoid it whenever possible.

Maybe we could use guards clauses here:

      def file_create(desc, f_path, f_content, f_mode = 00644)
        # [...]
        return resource unless resource.updated_by_last_action?
        new_resource.updated_by_last_action(resource.updated_by_last_action?)
        resource
      end

[karlsvec]

@zuazo: Thanks for the feedback, I've updated my branch accordingly.

[karlsvec]

@zuazo: Huge thanks for the help getting these changes to pass the tests.

As an aside, it seems like there should be a new release of the ssl_certificate cookbook, since the old behavior of the ssl_certificate resource is a bit "broken" with respect to its notification behavior.

[zuazo]

@karlsvec no problem. You are a great contributor. It is the least I can do.

I'm waiting for the integration tests to pass before releasing (CircleCI). Do you think minor bump will be enough?

[karlsvec]

@zuazo If by "minor" you mean version bump to 1.11.0, then yes, I agree that's a good idea. This commit was a small change, but it fixed a rather large bug in the ssl_certificate resource. Users are probably more likely to upgrade (and get the correct notification behavior) if they see that version change.

[zuazo]

Released in 1.11.0.

Thanks!