Skip to content

Conversation

chrishunt
Copy link
Contributor

See commits for details

@chrishunt
Copy link
Contributor Author

Dang. I see you just merged a refactor :)

@steveklabnik
Copy link
Owner

:'( Yeah. Sorry, I didn't know this was incoming!

@steveklabnik
Copy link
Owner

This might still be valuable, though: do those two tests fail without the rest of the patch?

@chrishunt
Copy link
Contributor Author

Those tests are required to verify https://github.com/steveklabnik/json-merge_patch/blob/master/lib/json/merge_patch.rb#L97

I just noticed that I could delete that line and nothing failed

@steveklabnik
Copy link
Owner

Right. Awesome. I had a feeling that I was missing some edge cases, but wasn't 100% sure.

@chrishunt
Copy link
Contributor Author

Let me rebase, master changed.

chrishunt added 3 commits May 19, 2013 09:53
There's no need to return nil for the special case obj.nil? since
returning the object itself will do.
@chrishunt
Copy link
Contributor Author

There you are sir

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 59b82e5 on chrishunt:chrishunt/refactor-merge into ae99592 on steveklabnik:master.

steveklabnik added a commit that referenced this pull request May 19, 2013
@steveklabnik steveklabnik merged commit df830ab into steveklabnik:master May 19, 2013
@steveklabnik
Copy link
Owner

Thank you very much!

@chrishunt
Copy link
Contributor Author

4.0 GPA 💯 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants