refactor: wrap rescue inside begin / end block

[theobarrague]

We are using Puppet 5 ( not a choice, not a choice ... ) and we need the latest version of module with safe_directory. I had to wrap rescue inside begin / end block to make it works.

[bastelfreak]

@theobarrague can you take a look at the ruvocop violations?

[theobarrague]

Hey @bastelfreak

I'm not sure (I'm not a Ruby developer) but the rescue block seems to work with Ruby 2.5+ only (changelog). With our Ruby version (2.4.10), it doesn't work without the "implicit" begin block. Without it, the code won't work:

Error: Could not autoload puppet/provider/vcsrepo/git: /opt/puppetlabs/puppet/cache/lib/puppet/provider/vcsrepo/git.rb:294: syntax error, unexpected keyword_rescue, expecting keyword_end
    rescue Puppet::ExecutionFailure
          ^
/opt/puppetlabs/puppet/cache/lib/puppet/provider/vcsrepo/git.rb:731: syntax error, unexpected keyword_end, expecting end-of-input
Error: Could not autoload puppet/type/vcsrepo: Could not autoload puppet/provider/vcsrepo/git: /opt/puppetlabs/puppet/cache/lib/puppet/provider/vcsrepo/git.rb:294: syntax error, unexpected keyword_rescue, expecting keyword_end
    rescue Puppet::ExecutionFailure
          ^
/opt/puppetlabs/puppet/cache/lib/puppet/provider/vcsrepo/git.rb:731: syntax error, unexpected keyword_end, expecting end-of-input
Error: Failed to apply catalog: Could not autoload puppet/type/vcsrepo: Could not autoload puppet/provider/vcsrepo/git: /opt/puppetlabs/puppet/cache/lib/puppet/provider/vcsrepo/git.rb:294: syntax error, unexpected keyword_rescue, expecting keyword_end
    rescue Puppet::ExecutionFailure
          ^
/opt/puppetlabs/puppet/cache/lib/puppet/provider/vcsrepo/git.rb:731: syntax error, unexpected keyword_end, expecting end-of-input

For the SLES-12, it seems failed installing puppet and not related to my PR

[theobarrague]

Hey @bastelfreak , is there anything wrong with that PR ?

[chelnak]

Hey @theobarrague, it looks like there are some rubocop violations still to resolve here.

I'm not entirely sure how much I agree with adding support back in for unsupported Ruby versions.

However, if the community decide this is ok to move forward then we should at least add rule ignores to the relevant lines.

[theobarrague]

@chelnak it's just a style issue ( Redundant 'begin' block detected https://www.rubydoc.info/gems/rubocop/0.27.0/RuboCop/Cop/Style/RedundantBegin ). This PR does not change the behavior of the code and make this module compatible with older ruby versions

[LukasAud]

This Rubocop failure seems to be a blocker for our unit tests. Obviously this is not ideal as we need our tests to be running properly if we want to be able monitor our modules. Either we add an exception to make Rubocop ignore the 'begin' lines (not ideal, we are already working on removing exceptions and outdated syntax) or we find an alternative.

What do you think, @chelnak?

[theobarrague]

@chelnak up 🙂

In my opinion, if you started to migrate old syntax to new syntax, close this PR.

Otherwise, if it's acceptable for you to have a few old syntax ( which don't change the behavior ) to support more versions of Puppet, merge ( i can add the rubocop rules ).

But in any case, please take a decision so i can continue with / without.

[CLAassistant]

All committers have signed the CLA.

[LukasAud]

Hey @theobarrague, sorry for the long delay in replying. I'm looking again at this and the Pull Request needs a rebase. If you can add the "redundant begin" rule exception to the rubocop_todo.yml (file introduced in our Puppet 8 update) alongside a small comment as to why it exists, I think we would be happy to merge this.

Also, I know sometimes its not possible but if you are still using Puppet 5, I would recommend updating to newer, supported versions 😄