Closed ghost closed 9 years ago
Hey @dg-ratiodata, thanks for the PR. There's some refactoring going on over in #34 and I'd like to get that merged before this because there will be some conflicts.
I'm going to add a few comments, but I'd recommend holding off on any changes until that other branch is merged.
I think the logic in generate_pattern
needs some reworking. As is, the only time the newly proposed opts[:patterns]
gets considered is if a specific generator is requested in which case having a subset of patterns passed doesn't make much sense.
If I were to use this feature, I would want to specify my 6 (or whatever) patterns I like and have it pick one from that list (but not randomly, as mentioned above).
Feel free to work on these things, but again much of this will need reworked anyhow after #34 is merged.
Thanks again!
Ok. I will live for now with the "vendored" gem and will send changes upstream again, when the refactoring is done.
BTW:
What do you think. If one sets "patterns" to %w(xes)
this will have the same effect than setting the generator
-option.
Feel free to work on these things, but again much of this will need reworked anyhow after #34 is merged.
I changed the implementation, but just to give you an idea.
Are there any plans to implement a "test"-suite? Any objections to rspec
.
What do you think. If one sets "patterns" to %w(xes) this will have the same effect than setting the generator-option.
This makes sense to me. I don't know why someone would want to specify a pool of one pattern to choose from, but if they do that should be the one they get.
Are there any plans to implement a "test"-suite? Any objections to rspec.
I think a test suite would be fantastic. If you're up for helping out there, rspec is fine with me.
Ok. After the refactoring? ;-)
Ok. After the refactoring? ;-)
That would be :metal:
Ok. I opened new PRs for that. You can close this one.
Thanks @maxmeyer. I will start looking at your PRs today hopefully.
You provide a large list of beautiful patterns. For my usecase I want to use just a subset of all available patterns. This PR makes it possible to reduce the available patterns as optional argument to ".generator".
BTW: Is there a test suite available anywhere?
Cheers, dg