Coordination - ensuring the gem is production-ready

[gonzalo-bulnes]

Hello,

I decided to tackle gonzalo-bulnes/dredd-rack#7, and to implement it using this gem in order to keep concerns separated. Before making dredd-hooks-ruby a dependency of Dredd::Rack, however, I'd like to refactor/improve it a bit so:

• its versions are named following the Semantic Versioning conventions
• it has a change log (example)
• its code "feels more like Ruby" - follows the usual naming conventions, is a bit more idiomatic, a bit DRYer...
• its maintenance is easier for a Ruby dev. - add a few Rake tasks for testing, eventually add a unit test suite

I'm happy to do that as a contribution to dredd-hooks-ruby, and opened this issue for coordination purpose.

More details

Some of that will be refactoring, but some will also introduce changes in the API (following the naming conventions may change the include DreddHooks::Methods statement for example), but to make it more familiar to Ruby developers.

What I see as the API:

• the binary name, and default URL, ports [can't be changed because Dredd relies on them - correct me if I'm wrong]
• the DSL [I see no reason to change it]
• the DSL inclusion statement [if I change it I'll make the change explicit - that's the point of semantic versioning]
• the hook names [no reason to change them, I mean it's a Dredd concern not a dredd-hooks-ruby one]

Questions / Ideas

I've forked the project, and will start working on it. I have a few questions:

1. I could formalize my TO DO list as a series of issues here (I can't open them against my fork), and close them from the corresponding commits. IMO it would allow to set a roadmap and they could be tagged accordingly or assigned to myself, but I want to be sure it's OK before flooding you with issues and PR
2. I'm assuming the current version being 0.0.1 makes changing the versioning scheme and slightly modifying the API not a big deal, but again, you are the maintainers and I don't want these changes to sound scary

What do you think?

Edit: I removed the third question because I was mistaken: Travis CI is installing the gem locally, and there is no inconvenient in bumping the version number without releasing the corresponding gem. I'll just start working in my fork, and am willing to PR the result as soon as you want. : )

[gonzalo-bulnes]

That's for the first step, a general review of style and tools. I haven't started renaming or DRYing anything yet.

The second step will be renaming things and removing duplication in a backward-compatible way, and I'll propose a few non-backward compatible changes as a third step.

[honzajavorek]

Hi @gonzalo-bulnes, I very much appreciate your effort to make dredd-ruby-hooks better! Unfortunately, @netmilk who's more skilled in Ruby is on vacation, but generally I like your proposal and I'm open to cooperate on getting your ideas integrated. To be honest, we do maintain this repository, but no extensive work or improvements are scheduled at this point, so any contributions are very welcome! 👌

Since I'm more of a Pythonista than a jeweller and @netmilk is not around, I'll have to generally trust you regarding your suggestions on what's idiomatic within Ruby ecosystem. Maybe we could also get some reviews from @abtris if he has some spare time 😉

So I'm giving you green and please, go ahead and file issues, send pull requests! 🎉

Changelog

There is some way we do create Changelog for Dredd. It would be nice to have the process unified. Please note I don't think the current state is completely ideal - apiaryio/dredd#440, suggestions on improvements welcome.

Coordination

What I see as the API

You correctly identified the interface and I have nothing to add to the break down you wrote.

I could formalize my TO DO list as a series of issues here

Please do!

version being 0.0.1 makes changing the versioning scheme and slightly modifying the API not a big deal

Exactly. I think in this case the version 0.0.1 basically denotes "initial implementation". I'd say current state of dredd-hooks-ruby codebase probably corresponds to the fact we created it to first see if people will use it at all and like the idea or not. It's more or less prototype. Since usefulness of hooks in various languages was, I believe, verified, I think we can continue with improvements and make it better now 👍

[gonzalo-bulnes]

Hi @honzajavorek,

Thanks for your detailed answer and for the green light! :D

I'll put together a summary of the changes I already made and post it here in the next few days. I'll also open a few issues to reflect the refactoring I've done so far and the few changes to come.

Note that I'm commenting each commit to ensure you can follow the refactoring even if not that familiar with Ruby, and commenting my intentions in each of the PR I opened against my fork. Anyway, I'll let you know when introducing more substantial changes, so you get a chance to step in.

Schedule

I started yesterday doing some preliminary refactoring before DRYing the DreddHooks::Methods and DreddHooks::Runner (work in progress). I'll discuss the changes I made in the corresponding PR against my fork to set some context and allow anyone to step into the discussion when they're ready.

In the next few days I plan to start DRYing these two modules quite a lot, in order to make the DSL extension as simple and cheap as possible.
Then I'll modularize a little bit the DreddHooks::Server for readability and rename a few things around (in a backward-compatible way).

That's going to take some time, and by then we should be able to discuss about the third step I mentioned earlier and about breaking backward compatibility in small ways, in order to make sure we're on the same page. : )

About the change log

I took a quick look at the references you mentioned and I think I got the idea. I'll start adding a few missing elements on the CHANGELOG I added to make it more similar to the other ones, and, again, we can talk about it at some point.

Cheers!

[gonzalo-bulnes]

Hi @honzajavorek,

I made a mistake an committed the entire refactoring with the wrong identity. It's fixed now, and I'm recreating the pull requests against my fork so you can review them when you're ready.

Also, I think it's simpler if I create one single PR for the first step (refactoring to make the project more idiomatic and DRYing the code). It will correspond to the first ten PR against my fork.

Then one single PR for the backward-compatible renaming tasks. And finally one single PR for the non-backward-compatible (but sensible IMHO) renaming tasks.

The three PR will reflect the different levels of "discussability" and reducing the review/discussion/merge process to three steps should make it less time-consuming for you as a team. What do you think?