From: Ben Keene <seraphire@gmail.com>
Date: Tue, 03 Dec 2019 16:03:31 GMT
Subject: Re: [PATCH v3 1/1] Python3 support for t9800 tests. Basic P4/Python3 support
Message-ID: <5c299c98-dda4-768d-7307-666bde85715d@gmail.com>
In-Reply-To: <20191203001854.GA59841@generichostname>


On 12/2/2019 7:18 PM, Denton Liu wrote:
> Hi Ben,
>
> Thanks for the contribution!
>
>> Subject: Python3 support for t9800 tests. Basic P4/Python3 support
> In git.git, the convention for commit subjects is to use
> "<area>: <summary>". Perhaps something like, "git-p4: support Python 3"?
> Although I doubt this patch should remain as is... More below.
I didn't realize the email message from gitgitgadget was going to be the 
commit message, I thought it was the PR message.  I'll work on changing 
that!
>
> On Mon, Dec 02, 2019 at 07:02:16PM +0000, Ben Keene via GitGitGadget wrote:
>> From: Ben Keene <seraphire@gmail.com>
> It would be nice to have a bit more information about what this patch
> does. Could you please fill this in with some more details about the
> whats and, more importantly, the _whys_ of your change?
Sure, I'll add more detail.
>> Signed-off-by: Ben Keene <seraphire@gmail.com>
>> ---
>>   git-p4.py | 825 +++++++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 628 insertions(+), 197 deletions(-)
> This is a very big change to be done in one patch. Could you please
> split this into multiple smaller patches that each do one logical
> change? For example, you could have the following series of changes:
>
> 	1. git-p4: use p4.exe if on Windows
> 	2. git-p4: introduce encoding helper functions # this is to
> 	        introduce the as_string(), as_bytes(), etc. functions
> 	3. git-p4: start using the encoding helper functions
> 	...
>
> This was just an example and you don't have to follow those literally. I
> just wanted to give you an idea of what I meant.
>
> You can see Documentation/SubmittingPatches#separate-commits for more
> information.
>
> Thanks,
>
> Denton
So my last question would be, should I open a different PR on 
gitgitgadget? I can cherry-pick my changes into another branch and 
restart my submission?