Add parent_for_notify.default_changed for repeated fields #26
Conversation
|
Thanks for tracking this down Peter. Just some question around the tests. Giving the new specs descriptive names would probably help a lot with confusion. |
| ) | ||
| end | ||
|
|
||
| c.value_for_tag?(1).should == true |
There was a problem hiding this comment.
i don't understand some of the content of these tests -- did you intend to set up the d protobuf and the c.e field but not test anything related to them?
There was a problem hiding this comment.
Yea I just wanted other fields set - I was debugging and created these tests as I went. I figure they don't hurt.
There was a problem hiding this comment.
I have to disagree with that -- they hurt in that it makes it more difficult for another developer to come along and understand what the tests are doing, the extra noise obfuscates things. I've been taking too long to merge this in though, so I'm going to merge it and get these cleaned up later.
|
Added some descriptions to the tests |
|
Can you bump to 1.5.1 after this? I know we talked about setting a reference to the commit but it would be so much easier for my repos to just have the bump :) |
Add parent_for_notify.default_changed for repeated fields
|
1.5.1 pushed to rubygems, thanks again |
No description provided.