Re: [RFC] Looking for opinions on accepting refactoring patches

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

Joey Pabalinas wrote:
> The reason is that many parts of the Zsh codebase are literally
> _byzantine_labyrinths_ of single-letter identifiers and uncommented
> shenanigans that were very confusing to figure out.

> But in my opinion this is probably why I haven't noticed as many people
> contributing to Zsh as I expect to (it really is an absolutely *amazing*

There may be other reasons but I'm sure it hasn't helped.

> piece of software engineering in my opinion), so I was interested in
> some comments about how patches concentrated only on refactoring the
> code with semantics changes whatsoever would be received?

I'm sure any improvements would be welcomed. Notions on good
refactorings can be subjective; such as breaking a long function into
smaller ones where the original long function was neatly divided
into self-contained blocks anyway. So as always, it depends on the
particulars of the patch.

> Is this something that would be encouraged? Or is the risk of bugs
> and regression just too heavy for this to be a realistic goal?

The best way to alleviate the risk of bugs is to add test cases at the
same time. If you're going through to make sense of the code, test cases
will occur to you naturally anyway. Running the existing tests with code
coverage enabled helps to see if code is getting any existing testing.

And if you're willing to fix what you break then I can't see that anyone
can have any complaints:

When I went through much of the guts of comparguments last year my main
aim was to fix a few bugs that had been bothering me for years rather
than refactoring. I'm usually reluctant to make quick and dirty fixes
without fully understanding the code. In that case, I accepted that I
would need to fix whatever I broke. That code might still be further
refactored but it is cleaner than it was. But, having added test cases
while going through to make sense of the logic, I'm now in a position
where I'm fairly confident that I can dabble in that part of the code
without breaking things.

It may also help if, when choosing what code to refactor, you have
a longer term view to some bugs you'd like to see squished or even
features that might be added.

Oliver