Feature/remove rspec mock warnings

[shirren]

Hi Martyn, I came across your gem which we are using on a commercial project. I'd like to contribute, can you please review my pull request, it's a starting point for me.

[mloughran]

Hi shirren, glad you're finding this useful.

One thing I would say is that if you when you're contributing to other people's code it's good practice not to make arbitrary syntactic changes, and to maintain a similar code style wherever possible. You've changed a lot (all?) of double quotes to single quotes, which inflates the diff unnecessarily, and is really a matter of taste. I had a quick look at the old code and double quotes seem most common (although it's not entirely consistent) so I would suggest that you go with the existing style.

I see you've also used a lot of post-1.8 syntax. I guess the time has come where dropping 1.8 is ok, so I'm fine with this.

What are those extra gems for also?

If you revert the quote changes (feel free to force push a history change), I'm happy to merge this once rspec 3.0 is released.

You're very welcome to make further changes, but I recommend opening an issue for discussion first - it helps avoid wasting time by doing things more than once :)

[shirren]

Hi Martyn,

Thanks for the feedback. Sorry, I should have discussed those changes before making them, that was my mistake. I myself am not a huge fan of stylistic changes for the sake of it. But I too noticed the inconsistencies, and figuring it was an open source project I added the Rubocop gem, and it reported that I should make those changes. So all the stylistic changes were made to appease Rubocop and achieve consistency. We can add exceptions to this like in the case of the double quotes.

Not sure how long it will be before Rspec 3 is released, it is at rc1 at the moment, so could be a month away, problem for me is that there are some other changes I would like to make to api_cache. and I'd rather not branch of master again.

As I am based in Sydney and you are in London the turn around time required to make these changes might not suit my employers. In this case do you think it is better if I fork the project and make the changes we require and keep it that way? Would this be your advice? I'd rather modify your gem and contribute to this project, but if the changes I make are not in keeping with the design goals you had in mind for api_cache collaboration might be tricky at best.

I am very interested to hear your thoughts on this matter, and by the way good work on authoring this gem in the first place, it's been a real time saver :-).

Kind Regards Shirren

[mloughran]

Hi Shirren,

Sorry for the delay getting back to you. I guess I don't really object to merging before 3.0 lands.

Looks like one can configure Rubocop to favour double quotes so if you could do that that would be great. I'm generally in favour of simplicity and minimising changes for their own sake - so if you could leave development gems like rubocop out of the repo that would be great.

I think it would be better to avoid a fork if possible. What kind of changes do you feel you need?

Martyn

[shirren]

Hi Martyn,

I've done as you suggested. The development gems bye bug and rubocop have been taken out. I've had to add a .rubocop.yml file for the exceptions. Also I've bumped the version number up because even though the current version is 0.2.4 it should be higher, as you've added some code since 0.2.4 as the version on ruby gems is out of sync with the one in github.

Kind Regards Shirren

[mloughran]

Thanks, looks good. I'll merge this later.

R.e. version numbering, FWIW I'm of the view that one shouldn't increment the version number until the point of release (it's assumed that master may be ahead of the version number), and the version number increment should be in the hands of the maintainer.

[shirren]

Hi Martyn,

That's a fair call I will roll that change back and leave to you to version and release.

Kind Regards Shirren

[shirren]

Hi Martyn,

Will you be merging this pull request soon. I'd like to base my work off this particular version, or is that not the correct thing to do?

Regards Shirren

[mloughran]

I made a few comments - in particular I'm not very keen on what looks like a lot of spec changes making things expectations which were previously stubs - unless I've misunderstood the new rspec syntax.

You can easily continue to work without waiting for me to merge. Just branch off this point and then rebase on top of master when this branch is merged. Makes sense?

[shirren]

Hi Martyn, I agree and those were mistakes on my part. I have changed those expect calls to use the more semantically correct allow, which is included as part of spec 3. I would rather not base any new work of this branch until you are happy with the approach or it could mean more work down the track, so happy for this work to be excepted and merged before proceeding.

[mloughran]

Hi Shirren. I'm sorry to disappoint you, but I won't be merging this branch. I wanted to do some refactoring to APICache today (including deleting a load of spec code which you modified), and just didn't feel that there was a compelling reason to merge what is a rather large changeset. Rspec 3 just doesn't provide any tangible benefits and I'm not keen on change for its own sake. Sorry for rather leading you up the garden path... but at least we now have the first new release in 3 years :)