Open Stack

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