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?

[honzajavorek]

@gonzalo-bulnes Sorry for not being responsive for a while. I'll try to get someone skilled in Ruby 💎 fast to review your work (I'm convinced it'll be awesome!). If everything goes well and if you don't mind, I'd consider to add you as a contributor to the repository, so the changes are easier to coordinate. You do a lot of very good work here and I feel my lack of dedication to dredd-hooks-ruby only slows you down in making the project better.

[gonzalo-bulnes]

Hi @honzajavorek,

Please don't worry about timings, you all do an awesome job creating all these tools and sometimes slowing down a little bit gives the occasion to make better decisions : )
Hopefully, the maintenance tasks should become easier with the recent refactorings.

I'd be glad to get contributor access to the repo, thanks for your trust!

At the moment, I'm writing a small example application so I can check that I'm not missing anything important to make the DreddHooks::Server usable as a library.

I'll keep you updated!

[gonzalo-bulnes]

I'll close this issue since coordination is now ensured in #13 : )

[gonzalo-bulnes]

Given its title, maybe I should keep this issue open until v1.0.0. Sorry for the confusion.

[gonzalo-bulnes]

@honzajavorek @netmilk I released v0.1.0, as a Semantic Versioning starting point.

No breaking changes from v0.0.1, and no new features either (CHANGELOG). In particular I didn't start adding documentation to the CLI itself, just to be sure not to put too much in a single release.

Now I'll start using Dredd Hooks in Dredd::Rack to properly check that the DreddHooks::Server can be used smoothly as a library (I don't expect big problems, but I still expect to highlight minor issues). From there I'll determine the next steps to fix any remaining issue and improve the user experience. If you have any comments, ideas, just let me know : )

[honzajavorek]

@gonzalo-bulnes AFAIK sem ver doesn't support anything below v1.0.0 and recommends to start versioning with v1.0.0, but I didn't check this against the spec.

👍 to all the work you did, it's really outstanding!

[gonzalo-bulnes]

Hi @honzajavorek,

Semantic Versioning considers the 0.x series as unstable versions:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable. (source)

The greatest value of Semantic Versioning is probably being able to automatically update dependencies in a safe way (in Ruby: being able to use '~> 1.0' as a version constraint and get automatic updates for the whole 1.x series with confidence that anything will break unexpectedly).

While it is true that the version constraints must be kept as strict as possible in the 0.x series (example: the current recommendation for '0.1.0' in our case, which means no one depending on the Dredd Hooks gem will see it updated without explicitly updating the constraint in their Gemfile), I still see value in setting milestones towards stabilizing the API.
The main benefit I see is that doing so allows to establish early communication with potential users and prevent uncomfortable elements to become part of the API.

I also found that the semver convention can easily be extended to make the communication a bit more expressive, and in my pre-1.0.0 projects I enforce the following addition:

As long as MAJOR is 0:

• PATCH releases SHOULD NOT (but MIGHT) introduce non-backward-compatible changes to the current API
• MINOR releases MAY introduce non-backward-compatible changes or not, but SHOULD display a visible warning in their README when they do. (It MAY happen, though, that such changes be released without warning, see semver.org)

That is optional (of course), fully compatible with the semver spec and any user ignoring this and referring to the spec would err on the safe side (assuming that anything can change anytime, and being cautious).
Even if not used explicitely, however, I find it provides a good guideline for communication and forces me to think early about what the project API is, and what it should be, while still allowing changes (and mistakes!) to be done.

That what I had in mind when saying "a semver starting point" : )

Note: Indeed, I never used this explicitely so far - I don't think it would help users to know the details, and the improvement in the communication should be perceptible without needing to agree on one more spec.

[honzajavorek]

@gonzalo-bulnes Okay! I totally dig moving towards sem ver 👍 I probably got just confused with the 0.x.x versions. As you write, it is valid sem ver, but sem rel had problems with it, so I probably just made an incorrect connection in my brain between those two.

[gonzalo-bulnes]

I think you cannot really automate anything before v1.0.0, right? That's probably why. (I'm not familiar with Semantic Release, but I saw the Dredd issue about it; it reads interesting.)