osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Python-Dev] Policy on refactoring/clean up


I know there was a big follow-up already, but I'd like to point out that
(while clearly not everyone feels the same) I am personally inclined to set
the bar pretty high for refactoring that don't add functionality. It makes
crawling through history using e.g. git blame harder, since the person who
last refactored the code ends up owning it even though they weren't
responsible for all its intricacies (which might separately be blame-able
on many different commits).

And TBH a desire to refactor a lot of code is often a sign of a relatively
new contributor who hasn't learned their way around the code yet, so they
tend to want to make the code follow their understanding rather than
letting their understanding follow the code.

Also see https://en.wikipedia.org/wiki/Wikipedia:Chesterton%27s_fence


On Tue, Jun 26, 2018 at 2:03 AM Jeroen Demeyer <J.Demeyer at ugent.be> wrote:

> Hello,
>
> On https://github.com/python/cpython/pull/7909 I encountered friction
> for a PR which I expected to be uncontroversial: it just moves some code
> without changing any functionality.
>
> So basically my question is: is there some CPython policy *against*
> refactoring code to make it easier to read and write? (Note that I'm not
> talking about pure style issues here)
>
> Background: cpython has a source file "call.c" (introduced in
> https://github.com/python/cpython/pull/12) but the corresponding
> declarations are split over several .h files. While working on PEP 580,
> I found this slightly confusing. I decided that it would make more sense
> to group all these declarations in a new file "call.h". That's what PR
> 7909 does. In my opinion, the resulting code is easier to read. It also
> defines a clear place for declarations of future functionality added to
> "call.c" (for example, if we add a public API for FASTCALL). Finally, I
> added/clarified a few comments.
>
> I expected the PR to be either ignored or accepted. However, I received
> a negative reaction from Inada Naoki on it.
>
> I don't mind closing the PR and keeping the status quo if there is a
> general agreement. However, I'm afraid that a future reviewer of PEP 580
> might say "your includes are a mess" and he will be right.
>
>
> Jeroen.
> _______________________________________________
> Python-Dev mailing list
> Python-Dev at python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/guido%40python.org
>


-- 
--Guido van Rossum (python.org/~guido)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20180626/a8718c46/attachment.html>