Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Be more explicit about passing a block#291

Closed
jfirebaugh wants to merge 1 commit into
rspec:masterfrom
bigfix:jruby_proc_fix
Closed

Be more explicit about passing a block#291
jfirebaugh wants to merge 1 commit into
rspec:masterfrom
bigfix:jruby_proc_fix

Conversation

@jfirebaugh

Copy link
Copy Markdown

JRuby 1.6.0 is not likely to support the previous bit of Proc.new magic; see http://jira.codehaus.org/browse/JRUBY-5420

JRuby 1.6.0 is not likely to support the previous bit of Proc.new
magic; see http://jira.codehaus.org/browse/JRUBY-5420
@jfirebaugh

Copy link
Copy Markdown
Author

@dchelimsky

Copy link
Copy Markdown
Contributor

I had it structured that way so the then clause (which happens more often than the else) wouldn't have to take the hit of Ruby creating a Proc object. That said, I did some benchmarks with this patch applied and not applied and there is no real difference on up to 100k examples on MRI or jruby.

@jfirebaugh

Copy link
Copy Markdown
Author

Be more explicit about passing a block

JRuby 1.6.0 is not likely to support the previous bit of Proc.new
magic; see http://jira.codehaus.org/browse/JRUBY-5420

@dchelimsky

Copy link
Copy Markdown
Contributor

Ugh - I ran the benchmarks without around hooks. Turns out that this change borks around hooks in jruby!

Leaving this open until I resolve that.

@jfirebaugh

Copy link
Copy Markdown
Author

Ugh, I see what you mean -- it breaks on JRuby 1.5.6:

NoMethodError: undefined method `metadata' for #Proc:0x18d1cbd

Looks like another JRuby bug.

@dchelimsky

Copy link
Copy Markdown
Contributor

I've got a fix ready - just running it through its paces against diff versions of ruby. Push coming soon.

@dchelimsky

Copy link
Copy Markdown
Contributor

I take it back. The fix I had was that Procsy no longer inherits from Proc, but it looks like it needs to in order to be able to use yield. This cuke is therefore failing: https://github.com/rspec/rspec-core/blob/master/features/hooks/around_hooks.feature#L12.

@jfirebaugh

Copy link
Copy Markdown
Author

Hmm... How's the performance of adding #metadata as a singleton method on a non-subclassed Proc instance?

@dchelimsky

Copy link
Copy Markdown
Contributor

Haven't tried and need to shut down for a few hours - wanna give that a roll and report back?

@dchelimsky

Copy link
Copy Markdown
Contributor

Make Procsy a module and extend the generated Procs instead of
inheriting from Proc.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants