temyers / cucumber-jvm-parallel-plugin

Maven plugin to help running Cucumber features in parallel
Apache License 2.0
129 stars 122 forks source link

Implement Tag expressions #165

Closed zdenek-jonas closed 6 years ago

zdenek-jonas commented 6 years ago

Hi, i have implement the current Cucumber tag expression. In this version cucumber Still supports both variants of tags. So i can totally remove the TagFilter. Just now it works by the same way as Cucumber. I have increase the Version of gherkin, so i have to rewrite some part of code. Thanks to use Cucumber itself, we do not need to test the Tag filtering more. It just now part of Cucumber projects.

The groovy test based on the filepath with "classpath". I have no time to fix these old tests. And this tests will be very quickly old, because the old tag behaviour is now deprecated.

Please give me a feedback to this code. I am open for it. have a nice day Zdenek Jonas

mpkorstanje commented 6 years ago

Thanks to use Cucumber itself, we do not need to test the Tag filtering more. It just now part of Cucumber projects.

While the plugin generates code for cucumber-jvm, it doesn't actually have a dependency on cucumber-jvm. This means the plugin will fail in a rather ugly manner when cucumber-jvm isn't on the class path.

Also the TagPredicate is an internal part of cucumber-jvm (it's not in the api package) so it may break unexpectedly.

Better to duplicate the code. That way it won't break.

zdenek-jonas commented 6 years ago

Sorry, I will need a more explanation what i should do. I use Cucumber-core. There is a Class TagPredicate. Should i copy this class(es) (functionality) to this plugin? Or is this way totally bad?

mpkorstanje commented 6 years ago

Copy it. And the tests.

zdenek-jonas commented 6 years ago

I will add the tests, but this i need to commit and i do not want to lose this thread.

mpkorstanje commented 6 years ago

Wow. Bit of a misunderstanding here.

I was only talking about the implementation of TagPredicate. Not the whole tag expression library. You can add that as a dependency.

zdenek-jonas commented 6 years ago

Ok... i am going to remove it....

zdenek-jonas commented 6 years ago

so now?

mpkorstanje commented 6 years ago

Looks much better! Can you remove the whitespace only changes and squash your commits? This is easy to do if you reset your branch and use a diff viewer to revert the chunks that are only whitespace.

Removing the whitespace changes makes it easier for people to see what you've actually changed. The squash makes the history a bit more understandable.

zdenek-jonas commented 6 years ago

Ok, whitespaces are out. But with the "squash" i have no experience. I have to study how to do it. Hopefully i will find something for eclipse.

mpkorstanje commented 6 years ago

https://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git

zdenek-jonas commented 6 years ago

Ok, Squash and whitespaces finished.

mpkorstanje commented 6 years ago

@temyers I've set things up for a new release (major version bump, migration notes, ect). Can you build it?

zdenek-jonas commented 6 years ago

Could you please release it, i need that.

mpkorstanje commented 6 years ago

It's unfunded open source. It moves at the speed of peoples spare time. You can always deploy your own version to your local nexus though. Just change the groupId to avoid confusion.

temyers commented 6 years ago

Thanks @mpkorstanje,

Releasing shortly.

temyers commented 6 years ago

5.0.0 released.

It should be available on Maven Central shortly.

zdenek-jonas commented 6 years ago

@temyers it is already in Mavenrepository. Thank you. @mpkorstanje Thank you for feedback and help with the implementation. It was my first open source pull request.

mpkorstanje commented 6 years ago

You're welcome!