Re: [PATCH v3 1/1] Python3 support for t9800 tests. Basic P4/Python3 support

On 12/2/2019 7:18 PM, Denton Liu wrote:

Hi Ben,

Thanks for the contribution!

quoted

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:

quoted

From: Ben Keene <redacted>

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.

quoted

Signed-off-by: Ben Keene <redacted>
---
  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?