Do people prefer pushing feature repos or one massive patch?
The implementation for PEP 488 is basically done (sans Windows installer stuff). I did the work in a features repo at https://hg.python.org/features/pep-488/ . Once I have addressed reviewer comments at http://bugs.python.org/issue23731 , would people prefer I simply push the features repo to hg.python.org/cpython and have the more granular history but have various "merge default" commits, or would people rather I do one massive commit?
On Wed, Apr 1, 2015, at 12:09, Brett Cannon wrote:
The implementation for PEP 488 is basically done (sans Windows installer stuff). I did the work in a features repo at https://hg.python.org/features/pep-488/ . Once I have addressed reviewer comments at http://bugs.python.org/issue23731 , would people prefer I simply push the features repo to hg.python.org/cpython and have the more granular history but have various "merge default" commits, or would people rather I do one massive commit?
I tend to prefer the one massive commit especially if there's a lot of "in progress" commits in the branch. It makes for cleaner and more-transactional history.
On Wed, Apr 1, 2015 at 12:38 PM Benjamin Peterson <benjamin@python.org> wrote:
On Wed, Apr 1, 2015, at 12:09, Brett Cannon wrote:
The implementation for PEP 488 is basically done (sans Windows installer stuff). I did the work in a features repo at https://hg.python.org/features/pep-488/ . Once I have addressed reviewer comments at http://bugs.python.org/issue23731 , would people prefer I simply push the features repo to hg.python.org/cpython and have the more granular history but have various "merge default" commits, or would people rather I do one massive commit?
I tend to prefer the one massive commit especially if there's a lot of "in progress" commits in the branch. It makes for cleaner and more-transactional history.
The commits are actually self-contained so that's not an issue in this case. But I do understand the desire for the easy rollback potential.
I like one massive patch, myself. :)
-- ~Ethan~
On 2 April 2015 at 04:09, Ethan Furman <ethan@stoneleaf.us> wrote:
I like one massive patch, myself. :)
Aye, I'm also in the "squash for the clean history" approach (FWIW, making this less of an either/or question is one of the benefits Gerrit offers over other code review systems, since you can combine posting a patch series with the "rebase if needed" setting for submitting approved changes. It's not a model Kallithea currently supports, but it's one I'd like to see it handle at some point in the future)
Regards, Nick.
-- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
I'm in the other camp.
The way I see it, a squash of history or massive patch file loses history. It loses details about the thought process of the implementer. It masks mistakes and obscures motivations. It also masks decisions made in the merge operation, further hiding potential problems.
On the other hand, a feature repo (or any separate series of commits), while retaining the history as it happened and thus the fidelity of the development, can always be mechanically reduced to a squashed patch (for review or other considerations, and in fact, the Python bug tracker will produce these squashed patches from feature repos automatically even if they're hosted in another system). Rollback is trivially easy; reverting a merge is as easy as reverting a squashed commit. It has the added benefit that any individual commit can be backed out automatically (in a squashed patch, that's not possible).
In other words, it's straightforward and easy to go from the latter model to the former, and generally impossible to reverse the operation.
In my opinion, it boils down to whether the group wants to restrict the options available for review. I would recommend that a contributor provide (or maintain) a feature repo if convenient.
-----Original Message----- From: python-committers [mailto:python-committers-bounces+jaraco=jaraco.com@python.org] On Behalf Of Nick Coghlan Sent: Thursday, 02 April, 2015 01:35 To: python-committers Subject: Re: [python-committers] Do people prefer pushing feature repos or one massive patch?
On 2 April 2015 at 04:09, Ethan Furman <ethan@stoneleaf.us> wrote:
I like one massive patch, myself. :)
Aye, I'm also in the "squash for the clean history" approach.
On Apr 02, 2015, at 12:06 PM, Jason R. Coombs wrote:
The way I see it, a squash of history or massive patch file loses history. It loses details about the thought process of the implementer. It masks mistakes and obscures motivations. It also masks decisions made in the merge operation, further hiding potential problems.
In general I agree. Coming from bzr, it's very rare that merges get rebased first, but bzr has a strong "mainline-of-development" view that tends to make squash-before-merge unnecessary. diffs, bisects, logs, etc generally follow first-parents by default so you don't see all the subcommits, unless you want to, which sometimes you do.
git doesn't really follow this tradition (although some commands have an option to follow first parents). Not sure about hg.
Cheers, -Barry
Where I come from we always squash. More detailed history is preserved in the code review tool (which keeps a snapshot every time you bounce it back to the reviewer). Looking at my own sub-commits when I'm working on a complex feature or bug fix, they are often checkpoints with no particular significance except that the code is syntactically correct, and a common reason for doing a sub-commit is when I've got to attend to something else (e.g. a meeting).
On Thu, Apr 2, 2015 at 6:31 AM, Barry Warsaw <barry@python.org> wrote:
On Apr 02, 2015, at 12:06 PM, Jason R. Coombs wrote:
The way I see it, a squash of history or massive patch file loses history. It loses details about the thought process of the implementer. It masks mistakes and obscures motivations. It also masks decisions made in the merge operation, further hiding potential problems.
In general I agree. Coming from bzr, it's very rare that merges get rebased first, but bzr has a strong "mainline-of-development" view that tends to make squash-before-merge unnecessary. diffs, bisects, logs, etc generally follow first-parents by default so you don't see all the subcommits, unless you want to, which sometimes you do.
git doesn't really follow this tradition (although some commands have an option to follow first parents). Not sure about hg.
Cheers, -Barry
python-committers mailing list python-committers@python.org https://mail.python.org/mailman/listinfo/python-committers
-- --Guido van Rossum (python.org/~guido)
On Thu, 02 Apr 2015 09:31:08 -0700, Guido van Rossum <guido@python.org> wrote:
Where I come from we always squash. More detailed history is preserved in the code review tool (which keeps a snapshot every time you bounce it back to the reviewer). Looking at my own sub-commits when I'm working on a complex feature or bug fix, they are often checkpoints with no particular significance except that the code is syntactically correct, and a common reason for doing a sub-commit is when I've got to attend to something else (e.g. a meeting).
I think a lot depends on the personal style of the committer. I don't do checkpoint commits, but only (try to do) commits where everything works and the tests pass, and the commit is reviewable as a single unit. I don't think there's a right or wrong way to do this, I think it depends on how the person doing it thinks and organizes their work best. I don't see a lot of value in preserving the history of someone who uses the checkpoint-commit style, but I do see potential value in preserving the history of someone who uses the atomic-commit style. Perhaps we should leave it up to the committer, based on that guideline? (Given our other preferences, I think a rebased commit would be the way to go if history is preserved.)
But, if we feel a need to pick just one, I'd pick squashed.
--David
On 2 April 2015 at 18:15, R. David Murray <rdmurray@bitdance.com> wrote:
On Thu, 02 Apr 2015 09:31:08 -0700, Guido van Rossum <guido@python.org> wrote:
Where I come from we always squash. More detailed history is preserved in the code review tool (which keeps a snapshot every time you bounce it back to the reviewer). Looking at my own sub-commits when I'm working on a complex feature or bug fix, they are often checkpoints with no particular significance except that the code is syntactically correct, and a common reason for doing a sub-commit is when I've got to attend to something else (e.g. a meeting).
I think a lot depends on the personal style of the committer. I don't do checkpoint commits, but only (try to do) commits where everything works and the tests pass, and the commit is reviewable as a single unit. I don't think there's a right or wrong way to do this, I think it depends on how the person doing it thinks and organizes their work best. I don't see a lot of value in preserving the history of someone who uses the checkpoint-commit style, but I do see potential value in preserving the history of someone who uses the atomic-commit style. Perhaps we should leave it up to the committer, based on that guideline? (Given our other preferences, I think a rebased commit would be the way to go if history is preserved.)
But, if we feel a need to pick just one, I'd pick squashed.
I tend to develop patches using Mercurial Queues, so I naturally produce squashed commits. On the other hand, I very frequently do checkpoint-style commits (sometimes even partially-completed work that doesn't pass tests) because I move between 2 PCs, and keeping work in progress just in the working directory isn't an option in that case.
Generally, I prefer squashed commits in any case. Paul
On 2 April 2015 at 22:06, Jason R. Coombs <jaraco@jaraco.com> wrote:
I'm in the other camp.
The way I see it, a squash of history or massive patch file loses history. It loses details about the thought process of the implementer. It masks mistakes and obscures motivations. It also masks decisions made in the merge operation, further hiding potential problems.
On the other hand, a feature repo (or any separate series of commits), while retaining the history as it happened and thus the fidelity of the development, can always be mechanically reduced to a squashed patch (for review or other considerations, and in fact, the Python bug tracker will produce these squashed patches from feature repos automatically even if they're hosted in another system). Rollback is trivially easy; reverting a merge is as easy as reverting a squashed commit. It has the added benefit that any individual commit can be backed out automatically (in a squashed patch, that's not possible).
In other words, it's straightforward and easy to go from the latter model to the former, and generally impossible to reverse the operation.
In my opinion, it boils down to whether the group wants to restrict the options available for review. I would recommend that a contributor provide (or maintain) a feature repo if convenient.
Having a feature repo to *work* on a patch is a great idea, I interpreted the question as being about what the mainline history should look like. The latter is where I strongly prefer the "atomic feature commit" model.
Cheers, Nick.
-- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
If you choose to merge, I would prefer that you rebase your changes before to avoid multiple merges. IMO the best to avoid merges at all :-)
Did someone review your large change?
Victor
2015-04-01 18:09 GMT+02:00 Brett Cannon <bcannon@gmail.com>:
The implementation for PEP 488 is basically done (sans Windows installer stuff). I did the work in a features repo at https://hg.python.org/features/pep-488/ . Once I have addressed reviewer comments at http://bugs.python.org/issue23731 , would people prefer I simply push the features repo to hg.python.org/cpython and have the more granular history but have various "merge default" commits, or would people rather I do one massive commit?
python-committers mailing list python-committers@python.org https://mail.python.org/mailman/listinfo/python-committers
On Wed, Apr 1, 2015 at 12:56 PM Victor Stinner <victor.stinner@gmail.com> wrote:
If you choose to merge, I would prefer that you rebase your changes before to avoid multiple merges. IMO the best to avoid merges at all :-)
It's sounding like one massive patch is the best option for people.
Did someone review your large change?
It just went up earlier today, so no. It's actually a fairly simple patch, it's just there was a lot of files involved that had some comment mentioning .pyo files.
-Brett
Victor
2015-04-01 18:09 GMT+02:00 Brett Cannon <bcannon@gmail.com>:
The implementation for PEP 488 is basically done (sans Windows installer stuff). I did the work in a features repo at https://hg.python.org/features/pep-488/ . Once I have addressed reviewer comments at http://bugs.python.org/issue23731 , would people prefer I simply push the features repo to hg.python.org/cpython and have the more granular history but have various "merge default" commits, or would people rather I do one massive commit?
python-committers mailing list python-committers@python.org https://mail.python.org/mailman/listinfo/python-committers
Hi,
FYI the faulthandler and tracemalloc were both added in a single commit, while they added a lot of new code and modified multiple files.
The development of faulthandler and tracemalloc started as third party projects on PyPI.
I almost rewrote tracemalloc from scratch while its PEP was discussed. I didn't want to keep the history, because for such task (update the implementation when the PEP changes), I use "hg commit" as I save a file in an editor. I don't care of having well formed and atomic changes. It's common to have a following "oops, fix ..." commit.
tracemalloc :
changeset: 87401:6e2089dbc5ad user: Victor Stinner <victor.stinner@gmail.com> date: Sat Nov 23 12:27:24 2013 +0100 files: Doc/library/debug.rst Doc/library/tracemalloc.rst Doc/license.rst Doc/using/cmd description: Issue #18874: Implement the PEP 454 (tracemalloc)
Doc/library/debug.rst | 1 + Doc/library/tracemalloc.rst | 608 +++++++++++++++ Doc/license.rst | 41 + Doc/using/cmdline.rst | 18 +- Lib/test/support/__init__.py | 19 + Lib/test/test_atexit.py | 4 +- Lib/test/test_capi.py | 2 +- Lib/test/test_threading.py | 4 +- Lib/test/test_tracemalloc.py | 797 ++++++++++++++++++++ Lib/tracemalloc.py | 464 +++++++++++ Modules/Setup.dist | 17 +- Modules/_tracemalloc.c | 1407 ++++++++++++++++++++++++++++++++++++ Modules/hashtable.c | 518 +++++++++++++ Modules/hashtable.h | 128 +++ PC/config.c | 2 + PCbuild/pythoncore.vcxproj | 5 +- Python/pythonrun.c | 4 + 17 files changed, 4024 insertions(+), 15 deletions(-)
faulthandler:
changeset: 69070:b0680b5a5215 user: Victor Stinner <victor.stinner@haypocalc.com> date: Thu Mar 31 01:31:06 2011 +0200 files: Doc/library/debug.rst Doc/library/faulthandler.rst Doc/usin description: Issue #11393: Add the new faulthandler module
Doc/library/debug.rst | 3 +- Doc/library/faulthandler.rst | 129 ++++ Doc/using/cmdline.rst | 7 + Doc/whatsnew/3.3.rst | 8 + Include/traceback.h | 40 + Lib/test/regrtest.py | 5 + Lib/test/script_helper.py | 5 +- Lib/test/test_faulthandler.py | 469 ++++++++++++++++ Misc/NEWS | 2 + Modules/Setup.dist | 3 + Modules/faulthandler.c | 971 ++++++++++++++++++++++++++++++++++ Modules/main.c | 1 + PC/config.c | 2 + PCbuild/pythoncore.vcproj | 4 + Python/pythonrun.c | 21 + Python/traceback.c | 235 ++++++++ configure | 2 +- configure.in | 2 +- pyconfig.h.in | 3 + 19 files changed, 1907 insertions(+), 5 deletions(-)
Victor
2015-04-01 18:09 GMT+02:00 Brett Cannon <bcannon@gmail.com>:
The implementation for PEP 488 is basically done (sans Windows installer stuff). I did the work in a features repo at https://hg.python.org/features/pep-488/ . Once I have addressed reviewer comments at http://bugs.python.org/issue23731 , would people prefer I simply push the features repo to hg.python.org/cpython and have the more granular history but have various "merge default" commits, or would people rather I do one massive commit?
python-committers mailing list python-committers@python.org https://mail.python.org/mailman/listinfo/python-committers
participants (10)
-
Barry Warsaw -
Benjamin Peterson -
Brett Cannon -
Ethan Furman -
Guido van Rossum -
Jason R. Coombs -
Nick Coghlan -
Paul Moore -
R. David Murray -
Victor Stinner