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

[Denton Liu <liu.denton@gmail.com>]

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.

> 
> 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"?

> 
> 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