[openstack-dev] [nova] patches that improve code quality

Shinobu Kinjo shinobu.kj at gmail.com
Wed Mar 9 22:44:21 UTC 2016


Ideally it's good practice to do refactoring the codes to add more
readability. In reality, it's kind of impossible.
Because there might be another process required after refactoring and
it would take a time.
But that's good point, honestly.

On Thu, Mar 10, 2016 at 1:33 AM, Matt Riedemann
<mriedem at linux.vnet.ibm.com> wrote:
>
>
> On 3/9/2016 6:22 AM, Radomir Dopieralski wrote:
>>
>> Some of the code we have in Nova is admittedly hard to read and brittle.
>> This is true even though we cave excellent code review,
>> and the reviewers that deeply care for the quality of code that is being
>> merged. Some of this code has been there always, some got that way as a
>> result of bug fixes and patches that were intended to touch as little
>> code as possible, and some is just to bad judgment or mistakes.
>>
>> Whatever the reason, such code accumulates and makes the program harder
>> to understand, to debug and to modify. The maintenance and development
>> costs rise. It's also less pleasant to work with such code base.
>>
>> Cleaning up such code and rewriting it to be easier to understand and
>> more robust is a good practice that every programmer should be familiar
>> with. However, it's not always easy or even possible to do that while
>> working on a feature or a bugfix. If our patch includes a lot of changes
>> that are unrelated to the feature or bug at hand, it becomes harder to
>> understand and review, and even runs a risk of being rejected. It's also
>> easier to introduce bugs that way.
>>
>> A good practice would be to make such changes in separate patches,
>> possibly also adding tests (where they are missing) showing that there
>> is no change in behavior. Such patches can be reviewed with special
>> attention to readability and robustness, and in the review there will be
>> often even further improvements. We end up with a better program and
>> easier (and more pleasant) work.
>>
>> And now, finally, I can get to the point of this e-mail. I'm relatively
>> new to this project, but I found no way to direct the (precious)
>> attention of core reviewers to such patches. They are not bugs, neither
>> they are parts of any new feature, and so there is no list in Nova that
>> core reviewers look at where we could add such a patch. Admittedly, such
>> patches are not urgent -- the code works the same (or almost the same)
>> way without them -- but it would be nice to have some way of merging
>> them eventually, because they do make our lives easier in the long run.
>>
>> So here's my question: what is the correct way to have such a patch
>> merged in Nova, and -- if there isn't one -- should we think about
>> making it easier?
>
>
> I don't think we give any special priority to technical debt cleanup unless
> it's part of a larger series, like cleaning up code before fixing a bug or
> landing a feature.
>
> So you can and should push patches to refactor code to make it more
> maintainable/readable/etc, but there isn't a guarantee that it's going to
> get attention any sooner than every other patch out in the review queue
> (according to stackalytics there are currently 842 open reviews in nova
> [1]).
>
> [1] http://stackalytics.com/report/reviews/nova/open
>
> --
>
> Thanks,
>
> Matt Riedemann
>
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



-- 
Email:
shinobu at linux.com
GitHub:
shinobu-x
Blog:
Life with Distributed Computational System based on OpenSource



More information about the OpenStack-dev mailing list