[openstack-dev] [magnum] versioned objects changes
Adrian Otto
adrian.otto at rackspace.com
Sat Aug 29 01:51:56 UTC 2015
We are going to merge this work. I understand and respect Hongbin's position, but I respectfully disagree. When we are presented with ways to implement low overhead best practices like versioned objects, we will. It's not that hard to bump the version of an object when you change it. I like having systemic enforcement of that.
On the subject of review 211057, if you submit a review to remove comments, that is purely stylistic in nature, then you are inviting a discussion of style with our reviewers, and deserve to make that patch stylistically perfect.
If that patch had actual code in it tat made Magnum better, and several reviewers voted against the format of the comments, that would be stupid, and I would +2 it in spite of any -1 votes as long as it meets our rules for submission (like it must have a bug number).
Finally, meaningful -1 votes are valuable, and should not be viewed as a waste of effort. That's what we do as a team to help each other continually improve, and to make Magnum something we can all be proud of. With all that said, if you only have a stylistic comment, that should be a -0 vote with a comment, not a -1. If you are making stylistic and material comments together, that's fine, use a -1 vote.
Thanks,
Adrian
On Aug 28, 2015, at 5:21 PM, Davanum Srinivas <davanum at gmail.com<mailto:davanum at gmail.com>> wrote:
Hongbin,
We are hearing the best advice available from the folks who started the library, evangelized it across nova, ironic, heat, neutron etc.
If we can spend so much time and energy (*FOUR* -1's on a review which just changes some commented lines - https://review.openstack.org/#/c/211057/) then we can and should clearly do better in things that really matter in the long run.
If we get into the rhythm of doing the right things and figuring out the steps needed right from the get go, it will pay off in the future.
My 2 cents.
Thanks,
Dims
PS: Note that i used "we" wearing my magnum core hat and not the o.vo/oslo core hat :)
On Fri, Aug 28, 2015 at 6:52 PM, Dan Smith <dms at danplanet.com<mailto:dms at danplanet.com>> wrote:
> If you want my inexperienced opinion, a young project is the perfect
> time to start this.
^--- This ---^
> I understand that something like [2] will cause a test to fail when you
> make a major change to a versioned object. But you *want* that. It helps
> reviewers more easily catch contributors to say "You need to update the
> version, because the hash changed". The sooner you start using versioned
> objects in the way they are designed, the smaller the upfront cost, and
> it will also be a major savings later on if something like [1] pops up.
...and the way it will be the least overhead is if it's part of the
culture of contributors and reviewers. It's infinitely harder to take
the culture shift after everyone is used to not having to think about
upgrades, not to mention the technical recovery Ryan mentioned.
It's not my call for Magnum, but long-term thinking definitely pays off
in this particular area.
--Dan
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe<http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe>
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
--
Davanum Srinivas :: https://twitter.com/dims
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: OpenStack-dev-request at lists.openstack.org<mailto:OpenStack-dev-request at lists.openstack.org>?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150829/5ea49292/attachment.html>
More information about the OpenStack-dev
mailing list