git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Joel Holdsworth <jholdsworth@nvidia.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/6] git-p4: Always pass cmd arguments to subprocess as a python lists
Date: Thu, 09 Dec 2021 14:42:15 -0800	[thread overview]
Message-ID: <xmqqo85po0xk.fsf@gitster.g> (raw)
In-Reply-To: <20211209201029.136886-2-jholdsworth@nvidia.com> (Joel Holdsworth's message of "Thu, 9 Dec 2021 20:10:24 +0000")

Joel Holdsworth <jholdsworth@nvidia.com> writes:

> Subject: Re: [PATCH 1/6] git-p4: Always pass cmd arguments to subprocess as a python lists

Style: downcase "Always" or anything that comes after the initial
"area" designator, i.e. "git-p4:".

The most important bit is missing from the proposed log message.

It says what the updated code does clearly (i.e. if we find a code
that still passes cmd arguments to subprocess not as a list, the
title tells us that this patch did not do a thorough job at it),
which is good.

But it does not explain why we want to do so.  There must be some
backstory about wanting to do so (e.g. "because it is more Python3
way of doing things") that we can explain to the future readers of
"git log" output.  Being aware of the general direction these
patches are taking us early would help the readers.

  reply	other threads:[~2021-12-09 22:42 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 20:10 [PATCH 0/6] Transition git-p4.py to support Python 3 only Joel Holdsworth
2021-12-09 20:10 ` [PATCH 1/6] git-p4: Always pass cmd arguments to subprocess as a python lists Joel Holdsworth
2021-12-09 22:42   ` Junio C Hamano [this message]
2021-12-09 20:10 ` [PATCH 2/6] git-p4: Don't print shell commands as " Joel Holdsworth
2021-12-09 20:10 ` [PATCH 3/6] git-p4: Removed support for Python 2 Joel Holdsworth
2021-12-09 22:44   ` Junio C Hamano
2021-12-09 23:07     ` rsbecker
2021-12-10  3:25   ` David Aguilar
2021-12-10 10:44     ` Joel Holdsworth
2021-12-09 20:10 ` [PATCH 4/6] git-p4: Decode byte strings before printing Joel Holdsworth
2021-12-09 22:47   ` Junio C Hamano
2021-12-10  8:40     ` Fabian Stelzer
2021-12-10 10:48       ` Joel Holdsworth
2021-12-10 10:41     ` Joel Holdsworth
2021-12-09 20:10 ` [PATCH 5/6] git-p4: Eliminate decode_stream and encode_stream Joel Holdsworth
2021-12-09 20:10 ` [PATCH 6/6] git-p4: Resolve RCS keywords in binary Joel Holdsworth
2021-12-10  7:57   ` Luke Diamand
2021-12-10 10:51     ` Joel Holdsworth
2021-12-10  0:48 ` [PATCH 0/6] Transition git-p4.py to support Python 3 only Ævar Arnfjörð Bjarmason
2021-12-10 10:37   ` Joel Holdsworth
2021-12-10 11:30     ` Ævar Arnfjörð Bjarmason
2021-12-10 21:34   ` Junio C Hamano
2021-12-10 21:53     ` rsbecker
2021-12-11 21:00     ` Elijah Newren
2021-12-12  8:55       ` Luke Diamand
2021-12-10  7:53 ` Luke Diamand
2021-12-10 10:54   ` Joel Holdsworth
2021-12-11  9:58     ` Luke Diamand
2021-12-13 13:47       ` Joel Holdsworth
2021-12-13 19:29         ` Junio C Hamano
2021-12-13 19:58           ` Joel Holdsworth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqo85po0xk.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jholdsworth@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).