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

Radomir Dopieralski openstack at sheep.art.pl
Wed Mar 9 12:22:56 UTC 2016


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?
-- 
Radomir Dopieralski




More information about the OpenStack-dev mailing list