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. On Mon, Dec 02, 2019 at 07:02:16PM +0000, Ben Keene via GitGitGadget wrote: > From: Ben Keene <seraphire@xxxxxxxxx> 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? > > Signed-off-by: Ben Keene <seraphire@xxxxxxxxx> > --- > 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
