PaperTrail.request { } should return the value of the block to aid in passive wrapping

[krainboltgreene]

So I just spent the last 3 hours debugging because (ultimately) I had overwritten a method that some library depended on and I assumed that the below would be passively wrapping:

def valid_for_authentication?(*)
  PaperTrail.request(whodunnit: "The Machine") do
    super
  end
end

Turns out it doesn't, because it's forced to return nil.

[jaredbeck]

Hi Kurtis. Looks good. Please:

1. Document the return type in the method documentation
2. Add a test
3. Add an entry to the changelog under Unreleased -> Added

[krainboltgreene]

@jaredbeck The whole point of this PR is that the return type could be anything.

[jaredbeck]

As I said, please put this under "Added". I don't see it as a breaking change.

Also, please omit the code example. The single sentence description is sufficient.

[krainboltgreene]

We're changing the return type of a public method from always nil to sometimes nil, are you sure that's not a breaking change?

[jaredbeck]

We're changing the return type of a public method from always nil to sometimes nil, are you sure that's not a breaking change?

Yes, you're right (both correct and righteous) to question this 😁 I don't consider it a breaking change, because I don't think anyone is depending on the fact that it is returning nil. What do you think? Do you think people will be depending on the nil?

[jaredbeck]

Document the return type in the method documentation

@jaredbeck The whole point of this PR is that the return type could be anything.

To clarify, I'd like the method documentation to say something like "If a block is given, returns value from block"

[krainboltgreene]

Roger.

[jaredbeck]

Still needs requested change to method documentation

[jaredbeck]

This is still listed under "Breaking Changes" and I still think it should be under "Added" for the reasons I have already given. I'm open to arguments either way, but I haven't heard any convincing arguments for it being a breaking change yet.

[jaredbeck]

Closing via #1077, preserving your authorship. Thanks Kurtis.