Re: [PATCH v4 02/11] git-p4: change the expansion test from basestring to list

From: Ben Keene <seraphire@gmail.com>
Date: Dec 5, 2019, 17:05 UTC
Message-ID: <446aa222-a26c-99b9-6078-60b3376d3e6d@gmail.com>
In-Reply-To: <20191205102724.GB315203@generichostname>

On 12/5/2019 5:27 AM, Denton Liu wrote:

Show 12 quoted lines

> Hi Ben,
>
> On Wed, Dec 04, 2019 at 10:29:28PM +0000, Ben Keene via GitGitGadget wrote:
>> From: Ben Keene <seraphire@gmail.com>
>>
>> Python 3+ handles strings differently than Python 2.7.
> Do you mean Python 3?
>
>> Since Python 2 is reaching it's end of life, a series of changes are being submitted to enable python 3.7+ support. The current code fails basic tests under python 3.7.
> Python 3.5 doesn't reach EOL until Q4 2020[1]. We should be testing
> these changes under 3.5 to ensure that we're not accidentally
> introducing stuff that's not backwards compatible.

I changed my commit text to say support for version 3.5 (which is actually the version I am running the test with).

Show 6 quoted lines

>> Change references to basestring in the isinstance tests to use list instead. This prepares the code to remove all references to basestring.
>>
>> The original code used basestring in a test to determine if a list or literal string was passed into 9 different functions.  This is used to determine if the shell should be evoked when calling subprocess methods.
> Once again, I'd swap the above two paragraphs. Problem then solution.
>
> Also, did you mean "invoked" instead of "evoked"?

Changed. And yes, I meant 'invoked'. I wasn't trying to make my code feel anything!

Show 8 quoted lines

>> Signed-off-by: Ben Keene <seraphire@gmail.com>
>> (cherry picked from commit 5b1b1c145479b5d5fd242122737a3134890409e6)
>> ---
>>   git-p4.py | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
> The patch itself looks good, though.
>
> [1]: https://devguide.python.org/#branchstatus