Enable warnings during spec execution and fix warnings

[ivoanjo]

This PR fixes all warnings from Hyperclient code during spec execution.

[dblock]

Can you rebase this? I am a little uncomfortable with the whole forward declaration thing though, is that avoidable?

[ivoanjo]

I've rebased with the current master 😄

As for the forward declaration, it indeed is a strange problem. In terms of opening classes twice, ruby has no troubles with it, and since Hyperclient doesn't do any strange meta things --- while it's being loaded no part of the code tries to use a Resource instance --- and thus it's a bit hard to get bitten, as you'd have to be doing really strange things such as requiring in a thread while trying to use hyperclient on other thread concurrently.

As for alternatives, the problem here is that we need to have something that the other files can check in order to stop the circular require. So we could set some kind of constant (instead of the class itself) that said "class has been / is in the process of being required", so that you'd then have

# in resource.rb
module Hyperclient
  RESOURCE_AVOID_CIRCULAR_REQUIRE = true
end

require 'some other file'

# in other files
require 'resource' unless defined? Hyperclient::RESOURCE_AVOID_CIRCULAR_REQUIRE

Another alternative would be instead of having a small predeclaration of the class at the beginning of the file, you could just move the requires to the end of the class (maybe leave a note at the top # note: requires at the bottom to avoid circular requires), and thus the whole class would be defined before the require operations.

This approach is feasible because you're not running any of the class code during definition, so it's ok to refer in advance to things that haven't been defined yet (you'll get an error if they aren't defined by the time the code tries to run).

Do you think any of these alternatives are better? I can also remove that commit too, if you're unconvinced with either 👍

[dblock]

I think either lib/hyperclient/resource.rb should not require anything and we just know that they are required elsewhere, or better, move all requires into a parent file, like hyperclient.rb?

[ivoanjo]

Ah, yes indeed. I'm a fan of "require what you use" (thus I didn't really think of just having it all in one place), but that approach would also solve the problem entirely. Shall I update the PR with that approach?

[dblock]

Yes please, that seems a lot cleaner.

[ivoanjo]

@dblock done 😄 and happy holidays ☃️

[dblock]

Merged, thank you.

[ivoanjo]

Awesome! Looking forward to the next release :)