Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 6 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0cf8813
Return the value of with
krainboltgreene Apr 12, 2018
0d7f125
Providing a detail for what changed
krainboltgreene Apr 12, 2018
8a28748
A simplistic test
krainboltgreene Apr 12, 2018
b1d842e
Removing example at request of owner
krainboltgreene Apr 12, 2018
c9a6348
Syntax error
krainboltgreene Apr 12, 2018
97a58ba
We have to provide options
krainboltgreene Apr 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).

### Breaking Changes

- None
- `PaperTrail.request do ... end` now returns whatever value is returned from the given block. For example:
``` ruby
raise PaperTrail.request(whodunnit: "Michael Bay") do
ApplicationError
end
# => ApplicationError has been raised!
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@krainboltgreene krainboltgreene Apr 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


### Added

Expand Down
1 change: 0 additions & 1 deletion lib/paper_trail.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ def request(options = nil, &block)
Request
else
Request.with(options, &block)
nil
end
end

Expand Down
6 changes: 6 additions & 0 deletions spec/paper_trail_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
require "spec_helper"

RSpec.describe PaperTrail do
describe ".request" do
it "returns the value returned by the block"
expect(described_class.request { "A test" }).to eq("A test")
end
end

describe "#config", versioning: true do
it "allows for config values to be set" do
expect(described_class.config.enabled).to eq(true)
Expand Down