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

Already on GitHub? Sign in to your account

Extract backtrace filtering methods into helper module. #701

Merged
merged 1 commit into from Oct 9, 2012

Conversation

Projects
None yet
3 participants
Owner

myronmarston commented Oct 5, 2012

This will be used by rspec/rspec-expectations#59.

@myronmarston myronmarston referenced this pull request in rspec/rspec-expectations Oct 5, 2012

Merged

Include backtrace in errors reported by `raise_error` matcher. #177

@dchelimsky dchelimsky and 1 other commented on an outdated diff Oct 5, 2012

lib/rspec/core/formatters/helpers.rb
@@ -3,6 +3,8 @@ module Core
module Formatters
module Helpers
+ extend self
@dchelimsky

dchelimsky Oct 5, 2012

Owner

I'd prefer to avoid this as I've seen this pattern cause lots of confusion in the past. We need to expose format_backtrace on something, so I'd recommend that we extract out the backtrace-specific material from this module and expose it in a separate module as module functions. Then this module can extend BacktraceFormatters (for example) and other consumers can invoke the functions directly. WDYT?

@myronmarston

myronmarston Oct 5, 2012

Owner

I'd prefer to avoid this as I've seen this pattern cause lots of confusion in the past.

Are you referring to extend self? It's one of my favorite ruby idioms and I use it frequently. What confusion have you seen it create?

Anyhow, I think it makes sense to put these methods in a module named BacktraceFormatter, as that'll be more intention-revealing when rspec-expectations uses it.

@dchelimsky

dchelimsky Oct 5, 2012

Owner

What confusion have you seen it create?

Methods with side effects - difficult to track down bugs. Probably not an issue w/ side-effect free functions.

myronmarston added a commit that referenced this pull request Oct 9, 2012

Merge pull request #701 from rspec/extract_backtrace_filtering
Extract backtrace filtering methods into helper module.

@myronmarston myronmarston merged commit 5ac165f into master Oct 9, 2012

1 check was pending

default The Travis build is in progress
Details

DouweM commented on 39857f3 Dec 1, 2012

FYI, changing the signature for #format_backtrace broke https://github.com/dgvncsz0f/rspec_formatters. I'm not sure if you follow semver, but if you do: #format_backtrace is a public API, so in a minor version update the signature should not have changed.

Owner

myronmarston replied Dec 1, 2012

@DouweM -- thanks for pointing this out. This was a completely unintentional API change. We try to follow SemVer, although there are places where we haven't documented very well if an API is public or not, so we're not 100% there.

I just pushed a fix to master to restore this API:

f06254c

I also plan to cut a 2.11.2 release (2.11.1 plus this fix) and a 2.12.1 release (2.12.0 plus this fix) to get the fixes out right away. I won't have the time tonight, though, but hopefully later this weekend.

Perfect, thanks for taking this seriously!

Owner

myronmarston replied Dec 1, 2012

@DouweM - I just released 2.12.1 with this fix (plus the other bug fixes that were already in master).

It turns out no 2.11.2 release was needed....2.11.1 didn't include the broken API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment