[openstack-dev] [nova] patches that improve code quality
mriedem at linux.vnet.ibm.com
Wed Mar 9 16:33:01 UTC 2016
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
More information about the OpenStack-dev