Remove the Ruby Gherkin parser. Rely on Go binary/protobuf #427
Conversation
|
@aslakhellesoy I've got the os/arch stuff handled, need to work out the executable stuff. You mentioned earlier about removing the s3 download backup so I haven't worked on that. Do you want that built for ruby, too? |
Bringing a few details into line with the latest Java code
There was a problem hiding this comment.
Great work! @mattwynne and I made some minor changes. Please go ahead and merge.
| @@ -2,7 +2,3 @@ | |||
| ../../.templates/github/ .github/ | |||
| ../../.templates/ruby/.travis.yml .travis.yml | |||
| ../testdata/ testdata/ | |||
| ../download-gherkin-go scripts/download-gherkin-go | |||
There was a problem hiding this comment.
Put this back in, so it gets updated if we make changes to ../download-gherkin-go
There was a problem hiding this comment.
Didn't look done, so I added this back on master.
There was a problem hiding this comment.
@aslakhellesoy Yeah, I had done it locally but hadn't pushed yet. Held off in favor of letting @enkessler have at it if he wanted, but for some reason my comment indicating that didn't make it? Womp. Thanks though!
| @@ -6,13 +6,11 @@ module Messages | |||
| describe SubprocessCucumberMessages do | |||
There was a problem hiding this comment.
Can you rename this class to Gherkin to bring it in line with the Java implementation? It makes for a nicer API.
There was a problem hiding this comment.
Hey, @enkessler, if you want this, you've got it. Let me know otherwise and I'll grab it. :)
There was a problem hiding this comment.
Actually, @enkessler we're thinking about naming it differently so belay last for now. Sorry!
There was a problem hiding this comment.
Since I've merged this to master now, let's just do this on master. First come first serve!
There was a problem hiding this comment.
TODO:
Let’s move `Gherkin::Messages::SubprocessCucumberMessages` to `Gherkin::Gherkin`
and `Gherkin::Messages::ProtobufCucumberMessages` to`Gherkin::ProtobufCucumberMessages`.
We don’t need the `Messages` module in the middle.
There was a problem hiding this comment.
@enkessler said he'll take a shot at it tonight.
|
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. |
Summary
Make way for Go! Getting rid of the ruby parser with @aslakhellesoy's help! (Thanks!)
Details
Delete all the things! Okay, not, ALL the things, but we've wiped out some code. I've set up the magic file for handle OS/Architecture detection and will eventually open to appropriate go executable.
Motivation and Context
Unify gherkin handling under one set of code rather than many involved language specific implementations
How Has This Been Tested?
Make ✅
TODO