search

cucumber / cucumber-ruby

Cucumber for Ruby. It's amazing!
https://cucumber.io
MIT License
5.18k stars 1.11k forks source link

"warning: `eval' should not be aliased" in JRuby build #432

Closed mattwynne closed 11 years ago

mattwynne commented 11 years ago

As witnessed in this build of master:

https://travis-ci.org/cucumber/cucumber/jobs/6201581

@os97673 I thought we'd fixed this?!

mattwynne commented 11 years ago

Also, fucking hell that JRuby build is slow!

Screen Shot 2013-04-10 at 00 09 27

os97673 commented 11 years ago

@os97673 I thought we'd fixed this?!

not exactly:

for now I've just introduced a new method and waiting the moment the old method can be removed

os97673 commented 11 years ago

Also, fucking hell that JRuby build is slow!

have you count how many time we start new cucumber? JRuby's startup is slow . Perhaps we could change our build/aruba to reuse the same JVM, but I don't know how to do this (yet).

aslakhellesoy commented 11 years ago

TagExpression is used by Cucumber in two places: Filtering out what scenarios to run based on --tags and filtering out whether or not to run a hook based on its tags.

This is Cucumber-internal usage, and I'd be surprised if it's used anywhere else.

If we remove TagExpression.eval we have to bump the minor version of gherkin so that cucumber users don't accidentally grab a gherkin that doesn't work with their cucumber.

os97673 commented 11 years ago

This is Cucumber-internal usage, and I'd be surprised if it's used anywhere else.

as we've learnt from cucumber 1.2.4 we may be surprised by the way people use our API ;)

If we remove TagExpression.eval we have to bump the minor version of gherkin so that cucumber users don't accidentally grab a gherkin that doesn't work with their cucumber.

yep, can we do this now? I mean is it ok to have 2.12.0 as a next gherkin version?

aslakhellesoy commented 11 years ago

Sure, no problem. Because of the circular dependency (yuck) things get a little complicated every time we bump gherkin minor, so be aware of that:

https://github.com/cucumber/gherkin/blob/master/gherkin.gemspec#L7-L19

aslakhellesoy commented 11 years ago

By that I meant go ahead and release it. I don't have time the next few days.

os97673 commented 11 years ago

Sure, no problem. Because of the circular dependency (yuck) things get a little complicated every time we bump gherkin minor, so be aware of that:

https://github.com/cucumber/gherkin/blob/master/gherkin.gemspec#L7-L19

this means that we have to release cucumber 1.3.0 and gherkin 2.12.0 together. I will target this ticket to 1.3.0 and will do it as the last change when we will be ready to release 1.3.0

aslakhellesoy commented 11 years ago

For cucumber 1.3.0/gherkin 2.12.0 I think we should remove TagExpression completely from gherkin and use the bool library instead to evaluate tag expressions. It's much better than what we currently have as people can use proper boolean expressions instead of that funky way we mimic AND, OR and NOT currently.

I'm in the process of switching cucumber-jvm to use bool, I'll post a link to the diff here when I'm done so you get an idea what's involved in this change.

mattwynne commented 11 years ago

Can I object and suggest we delay that until 1.4? We've already got 65 tickets going out in this release, I don't want to make it any bigger unless we have to.

aslakhellesoy commented 11 years ago

That's fine with me. I'll create a new ticket for switching to bool.

aslakhellesoy commented 11 years ago

More classes with an eval method that should be renamed to evaluate:

Plus the java equivalents.

os97673 commented 11 years ago

it looks like we do not use them directly in Cucumber, and they do not cause the warning (since there is no java implementation for them), this is way I've not renamed them before. But it looks like a good idea to rename them now to avoid similar problems in future.

lock[bot] commented 5 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.