* [PATCH 00/20] git-p4: Various code tidy-ups
@ 2022-01-12 13:46 Joel Holdsworth
2022-01-12 13:46 ` [PATCH 01/20] git-p4: add blank lines between functions and class definitions Joel Holdsworth
` (19 more replies)
0 siblings, 20 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
This patch set contains multiple patches to improve consistency and
tidyness of the git-p4 script's code style.
Many of these patches have been driven by the guidlines contained in the
Python PEP8 "Style Guide for Python Code" and were applied using a
mixture of human intervention, and tools including autopep8 and
pycodestyle.
This patch-set stops short of bringing git-p4 into full PEP8 compliance,
most notably in regard to the following items:
- There is no patch to apply the recommended column limit of
79-characters,
- There is no patch to correct hanging indents of multi-line
declarations such as multi-line function delcarations, function
invocations, etc.
Patches to correct these items may be provided later.
Joel Holdsworth (20):
git-p4: add blank lines between functions and class definitions
git-p4: remove unneeded semicolons from statements
git-p4: indent with 4-spaces
git-p4: improve consistency of docstring formatting
git-p4: convert descriptive class and function comments into
docstrings
git-p4: remove commented code
git-p4: sort and de-duplcate pylint disable list
git-p4: remove padding from lists, tuples and function arguments
git-p4: remove spaces around default arguments
git-p4: place a single space after every comma
git-p4: remove extraneous spaces before function arguments
git-p4: remove redundant backslash-continuations inside brackets
git-p4: remove spaces between dictionary keys and colons
git-p4: ensure every comment has a single #
git-p4: ensure there is a single space around all operators
git-p4: tidied visual indented lines of conditionals
git-p4: compare to singletons with "is" and "is not"
git-p4: only seperate code blocks by a single empty line
git-p4: move inline comments to line above
git-p4: seperate multiple statements onto seperate lines
git-p4.py | 851 ++++++++++++++++++++++++++++++++----------------------
1 file changed, 507 insertions(+), 344 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 01/20] git-p4: add blank lines between functions and class definitions
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 02/20] git-p4: remove unneeded semicolons from statements Joel Holdsworth
` (18 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
In the PEP8 style guidelines, top-level functions and class definitions
should be separated by two blank lines. Methods should be surrounded by
a single blank line.
This guideline is described here in the "Blank Lines" section:
https://www.python.org/dev/peps/pep-0008/#blank-lines
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 2 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index cb37545455..e6cc0f18a7 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -59,6 +59,7 @@
re_ko_keywords = re.compile(br'\$(Id|Header)(:[^$\n]+)?\$')
re_k_keywords = re.compile(br'\$(Id|Header|Author|Date|DateTime|Change|File|Revision)(:[^$\n]+)?\$')
+
def format_size_human_readable(num):
""" Returns a number of units (typically bytes) formatted as a human-readable
string.
@@ -71,6 +72,7 @@ def format_size_human_readable(num):
return "{:3.1f} {}B".format(num, unit)
return "{:.1f} YiB".format(num)
+
def p4_build_cmd(cmd):
"""Build a suitable p4 command line.
@@ -121,6 +123,7 @@ def p4_build_cmd(cmd):
return real_cmd
+
def git_dir(path):
""" Return TRUE if the given path is a git directory (/path/to/dir/.git).
This won't automatically add ".git" to a directory.
@@ -131,6 +134,7 @@ def git_dir(path):
else:
return d
+
def chdir(path, is_client_path=False):
"""Do chdir to the given path, and set the PWD environment
variable for use by P4. It does not look at getcwd() output.
@@ -153,6 +157,7 @@ def chdir(path, is_client_path=False):
path = os.getcwd()
os.environ['PWD'] = path
+
def calcDiskFree():
"""Return free space in bytes on the disk of the given dirname."""
if platform.system() == 'Windows':
@@ -163,6 +168,7 @@ def calcDiskFree():
st = os.statvfs(os.getcwd())
return st.f_bavail * st.f_frsize
+
def die(msg):
""" Terminate execution. Make sure that any running child processes have been wait()ed for before
calling this.
@@ -173,6 +179,7 @@ def die(msg):
sys.stderr.write(msg + "\n")
sys.exit(1)
+
def prompt(prompt_text):
""" Prompt the user to choose one of the choices
@@ -191,21 +198,25 @@ def prompt(prompt_text):
if response in choices:
return response
+
# We need different encoding/decoding strategies for text data being passed
# around in pipes depending on python version
if bytes is not str:
# For python3, always encode and decode as appropriate
def decode_text_stream(s):
return s.decode() if isinstance(s, bytes) else s
+
def encode_text_stream(s):
return s.encode() if isinstance(s, str) else s
else:
# For python2.7, pass read strings as-is, but also allow writing unicode
def decode_text_stream(s):
return s
+
def encode_text_stream(s):
return s.encode('utf_8') if isinstance(s, unicode) else s
+
def decode_path(path):
"""Decode a given string (bytes or otherwise) using configured path encoding options
"""
@@ -221,6 +232,7 @@ def decode_path(path):
print('Path with non-ASCII characters detected. Used {} to decode: {}'.format(encoding, path))
return path
+
def run_git_hook(cmd, param=[]):
"""Execute a hook if the hook exists."""
if verbose:
@@ -256,6 +268,7 @@ def run_git_hook(cmd, param=[]):
return run_hook_command(hook_file, param) == 0
+
def run_hook_command(cmd, param):
"""Executes a git hook command
cmd = the command line file to be executed. This can be
@@ -302,12 +315,14 @@ def write_pipe(c, stdin):
return val
+
def p4_write_pipe(c, stdin):
real_cmd = p4_build_cmd(c)
if bytes is not str and isinstance(stdin, str):
stdin = encode_text_stream(stdin)
return write_pipe(real_cmd, stdin)
+
def read_pipe_full(c):
""" Read output from command. Returns a tuple
of the return status, stdout text and stderr
@@ -321,6 +336,7 @@ def read_pipe_full(c):
(out, err) = p.communicate()
return (p.returncode, out, decode_text_stream(err))
+
def read_pipe(c, ignore_error=False, raw=False):
""" Read output from command. Returns the output text on
success. On failure, terminates execution, unless
@@ -338,6 +354,7 @@ def read_pipe(c, ignore_error=False, raw=False):
out = decode_text_stream(out)
return out
+
def read_pipe_text(c):
""" Read output from a command with trailing whitespace stripped.
On error, returns None.
@@ -348,10 +365,12 @@ def read_pipe_text(c):
else:
return decode_text_stream(out).rstrip()
+
def p4_read_pipe(c, ignore_error=False, raw=False):
real_cmd = p4_build_cmd(c)
return read_pipe(real_cmd, ignore_error, raw=raw)
+
def read_pipe_lines(c, raw=False):
if verbose:
sys.stderr.write('Reading pipe: %s\n' % str(c))
@@ -366,11 +385,13 @@ def read_pipe_lines(c, raw=False):
die('Command failed: %s' % str(c))
return lines
+
def p4_read_pipe_lines(c):
"""Specifically invoke p4 on the command supplied. """
real_cmd = p4_build_cmd(c)
return read_pipe_lines(real_cmd)
+
def p4_has_command(cmd):
"""Ask p4 for help on this command. If it returns an error, the
command does not exist in this version of p4."""
@@ -380,6 +401,7 @@ def p4_has_command(cmd):
p.communicate()
return p.returncode == 0
+
def p4_has_move_command():
"""See if the move command exists, that it supports -k, and that
it has not been administratively disabled. The arguments
@@ -400,6 +422,7 @@ def p4_has_move_command():
# assume it failed because @... was invalid changelist
return True
+
def system(cmd, ignore_error=False):
expand = not isinstance(cmd, list)
if verbose:
@@ -410,6 +433,7 @@ def system(cmd, ignore_error=False):
return retcode
+
def p4_system(cmd):
"""Specifically invoke p4 as the system command. """
real_cmd = p4_build_cmd(cmd)
@@ -418,9 +442,11 @@ def p4_system(cmd):
if retcode:
raise CalledProcessError(retcode, real_cmd)
+
def die_bad_access(s):
die("failure accessing depot: {0}".format(s.rstrip()))
+
def p4_check_access(min_expiration=1):
""" Check if we can access Perforce - account still logged in
"""
@@ -466,7 +492,10 @@ def p4_check_access(min_expiration=1):
else:
die_bad_access("unknown error code {0}".format(code))
+
_p4_version_string = None
+
+
def p4_version_string():
"""Read the version string, showing just the last line, which
hopefully is the interesting version bit.
@@ -482,12 +511,15 @@ def p4_version_string():
_p4_version_string = a[-1].rstrip()
return _p4_version_string
+
def p4_integrate(src, dest):
p4_system(["integrate", "-Dt", wildcard_encode(src), wildcard_encode(dest)])
+
def p4_sync(f, *options):
p4_system(["sync"] + list(options) + [wildcard_encode(f)])
+
def p4_add(f):
# forcibly add file names with wildcards
if wildcard_present(f):
@@ -495,29 +527,37 @@ def p4_add(f):
else:
p4_system(["add", f])
+
def p4_delete(f):
p4_system(["delete", wildcard_encode(f)])
+
def p4_edit(f, *options):
p4_system(["edit"] + list(options) + [wildcard_encode(f)])
+
def p4_revert(f):
p4_system(["revert", wildcard_encode(f)])
+
def p4_reopen(type, f):
p4_system(["reopen", "-t", type, wildcard_encode(f)])
+
def p4_reopen_in_change(changelist, files):
cmd = ["reopen", "-c", str(changelist)] + files
p4_system(cmd)
+
def p4_move(src, dest):
p4_system(["move", "-k", wildcard_encode(src), wildcard_encode(dest)])
+
def p4_last_change():
results = p4CmdList(["changes", "-m", "1"], skip_info=True)
return int(results[0]['change'])
+
def p4_describe(change, shelved=False):
"""Make sure it returns a valid result by checking for
the presence of field "time". Return a dict of the
@@ -546,6 +586,7 @@ def p4_describe(change, shelved=False):
return d
+
#
# Canonicalize the p4 type and return a tuple of the
# base type, plus any modifiers. See "p4 help filetypes"
@@ -581,6 +622,7 @@ def split_p4_type(p4type):
mods = s[1]
return (base, mods)
+
#
# return the raw p4 type of a file (text, text+ko, etc)
#
@@ -588,6 +630,7 @@ def p4_type(f):
results = p4CmdList(["fstat", "-T", "headType", wildcard_encode(f)])
return results[0]['headType']
+
#
# Given a type base and modifier, return a regexp matching
# the keywords that can be expanded in the file
@@ -603,6 +646,7 @@ def p4_keywords_regexp_for_type(base, type_mods):
else:
return None
+
#
# Given a file, return a regexp matching the possible
# RCS keywords that will be expanded, or None for files
@@ -615,6 +659,7 @@ def p4_keywords_regexp_for_file(file):
(type_base, type_mods) = split_p4_type(p4_type(file))
return p4_keywords_regexp_for_type(type_base, type_mods)
+
def setP4ExecBit(file, mode):
# Reopens an already open file and changes the execute bit to match
# the execute bit setting in the passed in mode.
@@ -630,6 +675,7 @@ def setP4ExecBit(file, mode):
p4_reopen(p4Type, file)
+
def getP4OpenedType(file):
# Returns the perforce file type for the given file.
@@ -640,6 +686,7 @@ def getP4OpenedType(file):
else:
die("Could not determine file type for %s (result: '%s')" % (file, result))
+
# Return the set of all p4 labels
def getP4Labels(depotPaths):
labels = set()
@@ -652,6 +699,7 @@ def getP4Labels(depotPaths):
return labels
+
# Return the set of all git tags
def getGitTags():
gitTags = set()
@@ -660,8 +708,10 @@ def getGitTags():
gitTags.add(tag)
return gitTags
+
_diff_tree_pattern = None
+
def parseDiffTreeEntry(entry):
"""Parses a single diff tree entry into its component elements.
@@ -699,41 +749,52 @@ def parseDiffTreeEntry(entry):
}
return None
+
def isModeExec(mode):
# Returns True if the given git mode represents an executable file,
# otherwise False.
return mode[-3:] == "755"
+
class P4Exception(Exception):
""" Base class for exceptions from the p4 client """
+
def __init__(self, exit_code):
self.p4ExitCode = exit_code
+
class P4ServerException(P4Exception):
""" Base class for exceptions where we get some kind of marshalled up result from the server """
+
def __init__(self, exit_code, p4_result):
super(P4ServerException, self).__init__(exit_code)
self.p4_result = p4_result
self.code = p4_result[0]['code']
self.data = p4_result[0]['data']
+
class P4RequestSizeException(P4ServerException):
""" One of the maxresults or maxscanrows errors """
+
def __init__(self, exit_code, p4_result, limit):
super(P4RequestSizeException, self).__init__(exit_code, p4_result)
self.limit = limit
+
class P4CommandException(P4Exception):
""" Something went wrong calling p4 which means we have to give up """
+
def __init__(self, msg):
self.msg = msg
def __str__(self):
return self.msg
+
def isModeExecChanged(src_mode, dst_mode):
return isModeExec(src_mode) != isModeExec(dst_mode)
+
def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
errors_as_exceptions=False):
@@ -819,6 +880,7 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
return result
+
def p4Cmd(cmd):
list = p4CmdList(cmd)
result = {}
@@ -826,6 +888,7 @@ def p4Cmd(cmd):
result.update(entry)
return result;
+
def p4Where(depotPath):
if not depotPath.endswith("/"):
depotPath += "/"
@@ -862,20 +925,25 @@ def p4Where(depotPath):
clientPath = clientPath[:-3]
return clientPath
+
def currentGitBranch():
return read_pipe_text(["git", "symbolic-ref", "--short", "-q", "HEAD"])
+
def isValidGitDir(path):
return git_dir(path) != None
+
def parseRevision(ref):
return read_pipe("git rev-parse %s" % ref).strip()
+
def branchExists(ref):
rev = read_pipe(["git", "rev-parse", "-q", "--verify", ref],
ignore_error=True)
return len(rev) > 0
+
def extractLogMessageFromGitCommit(commit):
logMessage = ""
@@ -890,6 +958,7 @@ def extractLogMessageFromGitCommit(commit):
logMessage += log
return logMessage
+
def extractSettingsGitLog(log):
values = {}
for line in log.split("\n"):
@@ -915,19 +984,24 @@ def extractSettingsGitLog(log):
values['depot-paths'] = paths.split(',')
return values
+
def gitBranchExists(branch):
proc = subprocess.Popen(["git", "rev-parse", branch],
stderr=subprocess.PIPE, stdout=subprocess.PIPE);
return proc.wait() == 0;
+
def gitUpdateRef(ref, newvalue):
subprocess.check_call(["git", "update-ref", ref, newvalue])
+
def gitDeleteRef(ref):
subprocess.check_call(["git", "update-ref", "-d", ref])
+
_gitConfig = {}
+
def gitConfig(key, typeSpecifier=None):
if key not in _gitConfig:
cmd = [ "git", "config" ]
@@ -938,6 +1012,7 @@ def gitConfig(key, typeSpecifier=None):
_gitConfig[key] = s.strip()
return _gitConfig[key]
+
def gitConfigBool(key):
"""Return a bool, using git config --bool. It is True only if the
variable is set to true, and False if set to false or not present
@@ -947,6 +1022,7 @@ def gitConfigBool(key):
_gitConfig[key] = gitConfig(key, '--bool') == "true"
return _gitConfig[key]
+
def gitConfigInt(key):
if key not in _gitConfig:
cmd = [ "git", "config", "--int", key ]
@@ -958,6 +1034,7 @@ def gitConfigInt(key):
_gitConfig[key] = None
return _gitConfig[key]
+
def gitConfigList(key):
if key not in _gitConfig:
s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
@@ -966,6 +1043,7 @@ def gitConfigList(key):
_gitConfig[key] = []
return _gitConfig[key]
+
def p4BranchesInGit(branchesAreInRemotes=True):
"""Find all the branches whose names start with "p4/", looking
in remotes or heads as specified by the argument. Return
@@ -998,6 +1076,7 @@ def p4BranchesInGit(branchesAreInRemotes=True):
return branches
+
def branch_exists(branch):
"""Make sure that the given ref name really exists."""
@@ -1010,6 +1089,7 @@ def branch_exists(branch):
# expect exactly one line of output: the branch name
return out.rstrip() == branch
+
def findUpstreamBranchPoint(head = "HEAD"):
branches = p4BranchesInGit()
# map from depot-path to branch name
@@ -1037,6 +1117,7 @@ def findUpstreamBranchPoint(head = "HEAD"):
return ["", settings]
+
def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent=True):
if not silent:
print("Creating/updating branch(es) in %s based on origin branch(es)"
@@ -1084,6 +1165,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
if update:
system("git update-ref %s %s" % (remoteHead, originHead))
+
def originP4BranchesExist():
return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
@@ -1097,12 +1179,14 @@ def p4ParseNumericChangeRange(parts):
return (changeStart, changeEnd)
+
def chooseBlockSize(blockSize):
if blockSize:
return blockSize
else:
return defaultBlockSize
+
def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
assert depotPaths
@@ -1180,6 +1264,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
changes = sorted(changes)
return changes
+
def p4PathStartsWith(path, prefix):
# This method tries to remedy a potential mixed-case issue:
#
@@ -1192,6 +1277,7 @@ def p4PathStartsWith(path, prefix):
return path.lower().startswith(prefix.lower())
return path.startswith(prefix)
+
def getClientSpec():
"""Look at the p4 client spec, create a View() object that contains
all the mappings, and return it."""
@@ -1222,6 +1308,7 @@ def getClientSpec():
return view
+
def getClientRoot():
"""Grab the client directory."""
@@ -1235,6 +1322,7 @@ def getClientRoot():
return entry["Root"]
+
#
# P4 wildcards are not allowed in filenames. P4 complains
# if you simply add them, but you can force it with "-f", in
@@ -1252,6 +1340,7 @@ def wildcard_decode(path):
.replace("%25", "%")
return path
+
def wildcard_encode(path):
# do % first to avoid double-encoding the %s introduced here
path = path.replace("%", "%25") \
@@ -1260,10 +1349,12 @@ def wildcard_encode(path):
.replace("@", "%40")
return path
+
def wildcard_present(path):
m = re.search("[*#@%]", path)
return m is not None
+
class LargeFileSystem(object):
"""Base class for large file system support."""
@@ -1345,6 +1436,7 @@ def processContent(self, git_mode, relPath, contents):
sys.stderr.write("%s moved to large file system (%s)\n" % (relPath, localLargeFile))
return (git_mode, contents)
+
class MockLFS(LargeFileSystem):
"""Mock large file system for testing."""
@@ -1368,6 +1460,7 @@ def pushFile(self, localLargeFile):
os.makedirs(remotePath)
shutil.copyfile(localLargeFile, os.path.join(remotePath, os.path.basename(localLargeFile)))
+
class GitLFS(LargeFileSystem):
"""Git LFS as backend for the git-p4 large file system.
See https://git-lfs.github.com/ for details."""
@@ -1456,6 +1549,7 @@ def processContent(self, git_mode, relPath, contents):
else:
return LargeFileSystem.processContent(self, git_mode, relPath, contents)
+
class Command:
delete_actions = ( "delete", "move/delete", "purge" )
add_actions = ( "add", "branch", "move/add" )
@@ -1471,6 +1565,7 @@ def ensure_value(self, attr, value):
setattr(self, attr, value)
return getattr(self, attr)
+
class P4UserMap:
def __init__(self):
self.userMapFromPerforceServer = False
@@ -1541,6 +1636,7 @@ def loadUserMapFromCache(self):
except IOError:
self.getUserMapFromPerforceServer()
+
class P4Submit(Command, P4UserMap):
conflict_behavior_choices = ("ask", "skip", "quit")
@@ -2545,6 +2641,7 @@ def run(self, args):
return True
+
class View(object):
"""Represent a p4 view ("p4 help views"), and map files in a
repo according to the view."""
@@ -2652,11 +2749,13 @@ def map_in_client(self, depot_path):
die( "Error: %s is not found in client spec path" % depot_path )
return ""
+
def cloneExcludeCallback(option, opt_str, value, parser):
# prepend "/" because the first "/" was consumed as part of the option itself.
# ("-//depot/A/..." becomes "/depot/A/..." after option parsing)
parser.values.cloneExclude += ["/" + re.sub(r"\.\.\.$", "", value)]
+
class P4Sync(Command, P4UserMap):
def __init__(self):
@@ -2885,7 +2984,6 @@ def encodeWithUTF8(self, path):
# output one file from the P4 stream
# - helper for streamP4Files
-
def streamOneP4File(self, file, contents):
file_path = file['depotFile']
relPath = self.stripRepoPath(decode_path(file_path), self.branchPrefixes)
@@ -3692,7 +3790,6 @@ def importHeadRevision(self, revision):
print("IO error details: {}".format(err))
print(self.gitError.read())
-
def importRevisions(self, args, branch_arg_given):
changes = []
@@ -4016,6 +4113,7 @@ def run(self, args):
return True
+
class P4Rebase(Command):
def __init__(self):
Command.__init__(self)
@@ -4052,6 +4150,7 @@ def rebase(self):
system("git diff-tree --stat --summary -M %s HEAD --" % oldHead)
return True
+
class P4Clone(P4Sync):
def __init__(self):
P4Sync.__init__(self)
@@ -4130,6 +4229,7 @@ def run(self, args):
return True
+
class P4Unshelve(Command):
def __init__(self):
Command.__init__(self)
@@ -4245,6 +4345,7 @@ def run(self, args):
return True
+
class P4Branches(Command):
def __init__(self):
Command.__init__(self)
@@ -4273,6 +4374,7 @@ def run(self, args):
print("%s <= %s (%s)" % (branch, ",".join(settings["depot-paths"]), settings["change"]))
return True
+
class HelpFormatter(optparse.IndentedHelpFormatter):
def __init__(self):
optparse.IndentedHelpFormatter.__init__(self)
@@ -4283,6 +4385,7 @@ def format_description(self, description):
else:
return ""
+
def printUsage(commands):
print("usage: %s <command> [options]" % sys.argv[0])
print("")
@@ -4291,6 +4394,7 @@ def printUsage(commands):
print("Try %s <command> --help for command specific help." % sys.argv[0])
print("")
+
commands = {
"submit" : P4Submit,
"commit" : P4Submit,
@@ -4301,6 +4405,7 @@ def printUsage(commands):
"unshelve" : P4Unshelve,
}
+
def main():
if len(sys.argv[1:]) == 0:
printUsage(commands.keys())
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 02/20] git-p4: remove unneeded semicolons from statements
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
2022-01-12 13:46 ` [PATCH 01/20] git-p4: add blank lines between functions and class definitions Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 03/20] git-p4: indent with 4-spaces Joel Holdsworth
` (17 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
Python allows the usage of compound statements where multiple statements
are written on a single line separared by semicolons. It is also
possible to add a semicolon after a single statement, however this is
generally considered to be untidy, and is unnecessary.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index e6cc0f18a7..cdf64df2b5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -886,7 +886,7 @@ def p4Cmd(cmd):
result = {}
for entry in list:
result.update(entry)
- return result;
+ return result
def p4Where(depotPath):
@@ -987,8 +987,8 @@ def extractSettingsGitLog(log):
def gitBranchExists(branch):
proc = subprocess.Popen(["git", "rev-parse", branch],
- stderr=subprocess.PIPE, stdout=subprocess.PIPE);
- return proc.wait() == 0;
+ stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+ return proc.wait() == 0
def gitUpdateRef(ref, newvalue):
@@ -3603,8 +3603,8 @@ def gitCommitByP4Change(self, ref, change):
def importNewBranch(self, branch, maxChange):
# make fast-import flush all changes to disk and update the refs using the checkpoint
# command so that we can try to find the branch parent in the git history
- self.gitStream.write("checkpoint\n\n");
- self.gitStream.flush();
+ self.gitStream.write("checkpoint\n\n")
+ self.gitStream.flush()
branchPrefix = self.depotPaths[0] + branch + "/"
range = "@1,%s" % maxChange
#print "prefix" + branchPrefix
@@ -3680,12 +3680,12 @@ def importChanges(self, changes, origin_revision=0):
fullBranch = self.projectName + branch
if fullBranch not in self.p4BranchesInGit:
if not self.silent:
- print("\n Importing new branch %s" % fullBranch);
+ print("\n Importing new branch %s" % fullBranch)
if self.importNewBranch(branch, change - 1):
parent = ""
self.p4BranchesInGit.append(fullBranch)
if not self.silent:
- print("\n Resuming with change %s" % change);
+ print("\n Resuming with change %s" % change)
if self.verbose:
print("parent determined through known branches: %s" % parent)
@@ -3753,7 +3753,7 @@ def importHeadRevision(self, revision):
% info['data'])
if info['data'].find("must refer to client") >= 0:
sys.stderr.write("This particular p4 error is misleading.\n")
- sys.stderr.write("Perhaps the depot path was misspelled.\n");
+ sys.stderr.write("Perhaps the depot path was misspelled.\n")
sys.stderr.write("Depot path: %s\n" % " ".join(self.depotPaths))
sys.exit(1)
if 'p4ExitCode' in info:
@@ -3861,7 +3861,7 @@ def openStreams(self):
self.importProcess = subprocess.Popen(["git", "fast-import"],
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
- stderr=subprocess.PIPE);
+ stderr=subprocess.PIPE)
self.gitOutput = self.importProcess.stdout
self.gitStream = self.importProcess.stdin
self.gitError = self.importProcess.stderr
@@ -4047,7 +4047,7 @@ def run(self, args):
self.loadUserMapFromCache()
self.labels = {}
if self.detectLabels:
- self.getLabels();
+ self.getLabels()
if self.detectBranches:
## FIXME - what's a P4 projectName ?
@@ -4133,9 +4133,9 @@ def run(self, args):
def rebase(self):
if os.system("git update-index --refresh") != 0:
- die("Some files in your working directory are modified and different than what is in your index. You can use git update-index <filename> to bring the index up to date or stash away all your changes with git stash.");
+ die("Some files in your working directory are modified and different than what is in your index. You can use git update-index <filename> to bring the index up to date or stash away all your changes with git stash.")
if len(read_pipe("git diff-index HEAD --")) > 0:
- die("You have uncommitted changes. Please commit them before rebasing or stash them away with git stash.");
+ die("You have uncommitted changes. Please commit them before rebasing or stash them away with git stash.")
[upstream, settings] = findUpstreamBranchPoint()
if len(upstream) == 0:
@@ -4436,7 +4436,7 @@ def main():
formatter = HelpFormatter())
try:
- (cmd, args) = parser.parse_args(sys.argv[2:], cmd);
+ (cmd, args) = parser.parse_args(sys.argv[2:], cmd)
except:
parser.print_help()
raise
@@ -4452,7 +4452,7 @@ def main():
if os.path.exists(cmd.gitdir):
cdup = read_pipe("git rev-parse --show-cdup").strip()
if len(cdup) > 0:
- chdir(cdup);
+ chdir(cdup)
if not isValidGitDir(cmd.gitdir):
if isValidGitDir(cmd.gitdir + "/.git"):
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 03/20] git-p4: indent with 4-spaces
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
2022-01-12 13:46 ` [PATCH 01/20] git-p4: add blank lines between functions and class definitions Joel Holdsworth
2022-01-12 13:46 ` [PATCH 02/20] git-p4: remove unneeded semicolons from statements Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 04/20] git-p4: improve consistency of docstring formatting Joel Holdsworth
` (16 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that all code should be indented in 4-space units. This
guideline is described here:
https://www.python.org/dev/peps/pep-0008/#indentation
Previously git-p4 had multiple cases where code was indented with a
non-multiple of 4-spaces. This patch fixes each of these.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index cdf64df2b5..5fb3e09875 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -950,12 +950,12 @@ def extractLogMessageFromGitCommit(commit):
## fixme: title is first line of commit, not 1st paragraph.
foundTitle = False
for log in read_pipe_lines(["git", "cat-file", "commit", commit]):
- if not foundTitle:
- if len(log) == 1:
- foundTitle = True
- continue
+ if not foundTitle:
+ if len(log) == 1:
+ foundTitle = True
+ continue
- logMessage += log
+ logMessage += log
return logMessage
@@ -1167,7 +1167,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent
def originP4BranchesExist():
- return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
+ return gitBranchExists("origin") or gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")
def p4ParseNumericChangeRange(parts):
@@ -2188,7 +2188,7 @@ def applyCommit(self, id):
submitTemplate = self.prepareLogMessage(template, logMessage, jobs)
if self.preserveUser:
- submitTemplate += "\n######## Actual user %s, modified after commit\n" % p4User
+ submitTemplate += "\n######## Actual user %s, modified after commit\n" % p4User
if self.checkAuthorship and not self.p4UserIsMe(p4User):
submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
@@ -2637,7 +2637,7 @@ def run(self, args):
# exit with error unless everything applied perfectly
if len(commits) != len(applied):
- sys.exit(1)
+ sys.exit(1)
return True
@@ -3472,7 +3472,7 @@ def guessProjectName(self):
p = p[:-1]
p = p[p.strip().rfind("/") + 1:]
if not p.endswith("/"):
- p += "/"
+ p += "/"
return p
def getBranchMapping(self):
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 04/20] git-p4: improve consistency of docstring formatting
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (2 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 03/20] git-p4: indent with 4-spaces Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 19:20 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 05/20] git-p4: convert descriptive class and function comments into docstrings Joel Holdsworth
` (15 subsequent siblings)
19 siblings, 1 reply; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
This patch attempts to improve the consistency of the docstrings by
making the following changes:
- Rewraps all docstrings to a 79-character column limit.
- Adds a full stop at the end of every docstring.
- Removes any spaces after the opening triple-quotes of all
docstrings.
- Sets the hanging indent of multi-line docstrings to 3-spaces.
- Ensures that the closing triple-quotes of multi-line docstrings are
always on a new line indented by a 3-space indent.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 309 +++++++++++++++++++++++++++++-------------------------
1 file changed, 167 insertions(+), 142 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 5fb3e09875..93f4ec843f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -61,9 +61,9 @@
def format_size_human_readable(num):
- """ Returns a number of units (typically bytes) formatted as a human-readable
- string.
- """
+ """Returns a number of units (typically bytes) formatted as a
+ human-readable string.
+ """
if num < 1024:
return '{:d} B'.format(num)
for unit in ["Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi"]:
@@ -76,10 +76,10 @@ def format_size_human_readable(num):
def p4_build_cmd(cmd):
"""Build a suitable p4 command line.
- This consolidates building and returning a p4 command line into one
- location. It means that hooking into the environment, or other configuration
- can be done more easily.
- """
+ This consolidates building and returning a p4 command line into one
+ location. It means that hooking into the environment, or other
+ configuration can be done more easily.
+ """
real_cmd = ["p4"]
user = gitConfig("git-p4.user")
@@ -125,9 +125,9 @@ def p4_build_cmd(cmd):
def git_dir(path):
- """ Return TRUE if the given path is a git directory (/path/to/dir/.git).
- This won't automatically add ".git" to a directory.
- """
+ """Return TRUE if the given path is a git directory (/path/to/dir/.git).
+ This won't automatically add ".git" to a directory.
+ """
d = read_pipe(["git", "--git-dir", path, "rev-parse", "--git-dir"], True).strip()
if not d or len(d) == 0:
return None
@@ -136,20 +136,18 @@ def git_dir(path):
def chdir(path, is_client_path=False):
- """Do chdir to the given path, and set the PWD environment
- variable for use by P4. It does not look at getcwd() output.
- Since we're not using the shell, it is necessary to set the
- PWD environment variable explicitly.
-
- Normally, expand the path to force it to be absolute. This
- addresses the use of relative path names inside P4 settings,
- e.g. P4CONFIG=.p4config. P4 does not simply open the filename
- as given; it looks for .p4config using PWD.
-
- If is_client_path, the path was handed to us directly by p4,
- and may be a symbolic link. Do not call os.getcwd() in this
- case, because it will cause p4 to think that PWD is not inside
- the client path.
+ """Do chdir to the given path, and set the PWD environment variable for use
+ by P4. It does not look at getcwd() output. Since we're not using the
+ shell, it is necessary to set the PWD environment variable explicitly.
+
+ Normally, expand the path to force it to be absolute. This addresses
+ the use of relative path names inside P4 settings, e.g.
+ P4CONFIG=.p4config. P4 does not simply open the filename as given; it
+ looks for .p4config using PWD.
+
+ If is_client_path, the path was handed to us directly by p4, and may be
+ a symbolic link. Do not call os.getcwd() in this case, because it will
+ cause p4 to think that PWD is not inside the client path.
"""
os.chdir(path)
@@ -170,9 +168,9 @@ def calcDiskFree():
def die(msg):
- """ Terminate execution. Make sure that any running child processes have been wait()ed for before
- calling this.
- """
+ """Terminate execution. Make sure that any running child processes have
+ been wait()ed for before calling this.
+ """
if verbose:
raise Exception(msg)
else:
@@ -181,11 +179,11 @@ def die(msg):
def prompt(prompt_text):
- """ Prompt the user to choose one of the choices
+ """Prompt the user to choose one of the choices
- Choices are identified in the prompt_text by square brackets around
- a single letter option.
- """
+ Choices are identified in the prompt_text by square brackets around a
+ single letter option.
+ """
choices = set(m.group(1) for m in re.finditer(r"\[(.)\]", prompt_text))
while True:
sys.stderr.flush()
@@ -218,8 +216,10 @@ def encode_text_stream(s):
def decode_path(path):
- """Decode a given string (bytes or otherwise) using configured path encoding options
- """
+ """Decode a given string (bytes or otherwise) using configured path
+ encoding options.
+ """
+
encoding = gitConfig('git-p4.pathEncoding') or 'utf_8'
if bytes is not str:
return path.decode(encoding, errors='replace') if isinstance(path, bytes) else path
@@ -271,20 +271,20 @@ def run_git_hook(cmd, param=[]):
def run_hook_command(cmd, param):
"""Executes a git hook command
- cmd = the command line file to be executed. This can be
- a file that is run by OS association.
- param = a list of parameters to pass to the cmd command
+ cmd -- the command line file to be executed. This can be a file that
+ is run by OS association.
+
+ param -- a list of parameters to pass to the cmd command
- On windows, the extension is checked to see if it should
- be run with the Git for Windows Bash shell. If there
- is no file extension, the file is deemed a bash shell
- and will be handed off to sh.exe. Otherwise, Windows
- will be called with the shell to handle the file assocation.
+ On Windows, the extension is checked to see if it should be run with
+ the Git for Windows Bash shell. If there is no file extension, the file
+ is deemed a bash shell and will be handed off to sh.exe. Otherwise,
+ Windows will be called with the shell to handle the file assocation.
For non Windows operating systems, the file is called
as an executable.
- """
+ """
cli = [cmd] + param
use_shell = False
if platform.system() == 'Windows':
@@ -324,10 +324,9 @@ def p4_write_pipe(c, stdin):
def read_pipe_full(c):
- """ Read output from command. Returns a tuple
- of the return status, stdout text and stderr
- text.
- """
+ """Read output from command. Returns a tuple of the return status, stdout
+ text and stderr text.
+ """
if verbose:
sys.stderr.write('Reading pipe: %s\n' % str(c))
@@ -338,12 +337,12 @@ def read_pipe_full(c):
def read_pipe(c, ignore_error=False, raw=False):
- """ Read output from command. Returns the output text on
- success. On failure, terminates execution, unless
- ignore_error is True, when it returns an empty string.
+ """Read output from command. Returns the output text on success. On
+ failure, terminates execution, unless ignore_error is True, when it
+ returns an empty string.
- If raw is True, do not attempt to decode output text.
- """
+ If raw is True, do not attempt to decode output text.
+ """
(retcode, out, err) = read_pipe_full(c)
if retcode != 0:
if ignore_error:
@@ -356,9 +355,9 @@ def read_pipe(c, ignore_error=False, raw=False):
def read_pipe_text(c):
- """ Read output from a command with trailing whitespace stripped.
- On error, returns None.
- """
+ """Read output from a command with trailing whitespace stripped. On error,
+ returns None.
+ """
(retcode, out, err) = read_pipe_full(c)
if retcode != 0:
return None
@@ -387,14 +386,15 @@ def read_pipe_lines(c, raw=False):
def p4_read_pipe_lines(c):
- """Specifically invoke p4 on the command supplied. """
+ """Specifically invoke p4 on the command supplied."""
real_cmd = p4_build_cmd(c)
return read_pipe_lines(real_cmd)
def p4_has_command(cmd):
- """Ask p4 for help on this command. If it returns an error, the
- command does not exist in this version of p4."""
+ """Ask p4 for help on this command. If it returns an error, the command
+ does not exist in this version of p4.
+ """
real_cmd = p4_build_cmd(["help", cmd])
p = subprocess.Popen(real_cmd, stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
@@ -403,10 +403,11 @@ def p4_has_command(cmd):
def p4_has_move_command():
- """See if the move command exists, that it supports -k, and that
- it has not been administratively disabled. The arguments
- must be correct, but the filenames do not have to exist. Use
- ones with wildcards so even if they exist, it will fail."""
+ """See if the move command exists, that it supports -k, and that it has not
+ been administratively disabled. The arguments must be correct, but the
+ filenames do not have to exist. Use ones with wildcards so even if they
+ exist, it will fail.
+ """
if not p4_has_command("move"):
return False
@@ -435,7 +436,7 @@ def system(cmd, ignore_error=False):
def p4_system(cmd):
- """Specifically invoke p4 as the system command. """
+ """Specifically invoke p4 as the system command."""
real_cmd = p4_build_cmd(cmd)
expand = not isinstance(real_cmd, list)
retcode = subprocess.call(real_cmd, shell=expand)
@@ -448,8 +449,8 @@ def die_bad_access(s):
def p4_check_access(min_expiration=1):
- """ Check if we can access Perforce - account still logged in
- """
+ """Check if we can access Perforce - account still logged in."""
+
results = p4CmdList(["login", "-s"])
if len(results) == 0:
@@ -497,14 +498,14 @@ def p4_check_access(min_expiration=1):
def p4_version_string():
- """Read the version string, showing just the last line, which
- hopefully is the interesting version bit.
+ """Read the version string, showing just the last line, which hopefully is
+ the interesting version bit.
$ p4 -V
Perforce - The Fast Software Configuration Management System.
Copyright 1995-2011 Perforce Software. All rights reserved.
Rev. P4/NTX86/2011.1/393975 (2011/12/16).
- """
+ """
global _p4_version_string
if not _p4_version_string:
a = p4_read_pipe_lines(["-V"])
@@ -559,9 +560,11 @@ def p4_last_change():
def p4_describe(change, shelved=False):
- """Make sure it returns a valid result by checking for
- the presence of field "time". Return a dict of the
- results."""
+ """Make sure it returns a valid result by checking for the presence of
+ field "time".
+
+ Return a dict of the results.
+ """
cmd = ["describe", "-s"]
if shelved:
@@ -715,21 +718,22 @@ def getGitTags():
def parseDiffTreeEntry(entry):
"""Parses a single diff tree entry into its component elements.
- See git-diff-tree(1) manpage for details about the format of the diff
- output. This method returns a dictionary with the following elements:
-
- src_mode - The mode of the source file
- dst_mode - The mode of the destination file
- src_sha1 - The sha1 for the source file
- dst_sha1 - The sha1 fr the destination file
- status - The one letter status of the diff (i.e. 'A', 'M', 'D', etc)
- status_score - The score for the status (applicable for 'C' and 'R'
- statuses). This is None if there is no score.
- src - The path for the source file.
- dst - The path for the destination file. This is only present for
- copy or renames. If it is not present, this is None.
-
- If the pattern is not matched, None is returned."""
+ See git-diff-tree(1) manpage for details about the format of the diff
+ output. This method returns a dictionary with the following elements:
+
+ src_mode - The mode of the source file
+ dst_mode - The mode of the destination file
+ src_sha1 - The sha1 for the source file
+ dst_sha1 - The sha1 fr the destination file
+ status - The one letter status of the diff (i.e. 'A', 'M', 'D', etc)
+ status_score - The score for the status (applicable for 'C' and 'R'
+ statuses). This is None if there is no score.
+ src - The path for the source file.
+ dst - The path for the destination file. This is only present for
+ copy or renames. If it is not present, this is None.
+
+ If the pattern is not matched, None is returned.
+ """
global _diff_tree_pattern
if not _diff_tree_pattern:
@@ -757,14 +761,16 @@ def isModeExec(mode):
class P4Exception(Exception):
- """ Base class for exceptions from the p4 client """
+ """Base class for exceptions from the p4 client."""
def __init__(self, exit_code):
self.p4ExitCode = exit_code
class P4ServerException(P4Exception):
- """ Base class for exceptions where we get some kind of marshalled up result from the server """
+ """Base class for exceptions where we get some kind of marshalled up result
+ from the server.
+ """
def __init__(self, exit_code, p4_result):
super(P4ServerException, self).__init__(exit_code)
@@ -774,7 +780,7 @@ def __init__(self, exit_code, p4_result):
class P4RequestSizeException(P4ServerException):
- """ One of the maxresults or maxscanrows errors """
+ """One of the maxresults or maxscanrows errors."""
def __init__(self, exit_code, p4_result, limit):
super(P4RequestSizeException, self).__init__(exit_code, p4_result)
@@ -782,7 +788,7 @@ def __init__(self, exit_code, p4_result, limit):
class P4CommandException(P4Exception):
- """ Something went wrong calling p4 which means we have to give up """
+ """Something went wrong calling p4 which means we have to give up."""
def __init__(self, msg):
self.msg = msg
@@ -1016,7 +1022,8 @@ def gitConfig(key, typeSpecifier=None):
def gitConfigBool(key):
"""Return a bool, using git config --bool. It is True only if the
variable is set to true, and False if set to false or not present
- in the config."""
+ in the config.
+ """
if key not in _gitConfig:
_gitConfig[key] = gitConfig(key, '--bool') == "true"
@@ -1049,7 +1056,8 @@ def p4BranchesInGit(branchesAreInRemotes=True):
in remotes or heads as specified by the argument. Return
a dictionary of { branch: revision } for each one found.
The branch names are the short names, without any
- "p4/" prefix."""
+ "p4/" prefix.
+ """
branches = {}
@@ -1280,7 +1288,8 @@ def p4PathStartsWith(path, prefix):
def getClientSpec():
"""Look at the p4 client spec, create a View() object that contains
- all the mappings, and return it."""
+ all the mappings, and return it.
+ """
specList = p4CmdList("client -o")
if len(specList) != 1:
@@ -1363,13 +1372,15 @@ def __init__(self, writeToGitStream):
self.writeToGitStream = writeToGitStream
def generatePointer(self, cloneDestination, contentFile):
- """Return the content of a pointer file that is stored in Git instead of
- the actual content."""
+ """Return the content of a pointer file that is stored in Git instead
+ of the actual content.
+ """
assert False, "Method 'generatePointer' required in " + self.__class__.__name__
def pushFile(self, localLargeFile):
"""Push the actual content which is not stored in the Git repository to
- a server."""
+ a server.
+ """
assert False, "Method 'pushFile' required in " + self.__class__.__name__
def hasLargeFileExtension(self, relPath):
@@ -1417,7 +1428,8 @@ def isLargeFile(self, relPath):
def processContent(self, git_mode, relPath, contents):
"""Processes the content of git fast import. This method decides if a
file is stored in the large file system and handles all necessary
- steps."""
+ steps.
+ """
if self.exceedsLargeFileThreshold(relPath, contents) or self.hasLargeFileExtension(relPath):
contentTempFile = self.generateTempFile(contents)
(pointer_git_mode, contents, localLargeFile) = self.generatePointer(contentTempFile)
@@ -1442,7 +1454,8 @@ class MockLFS(LargeFileSystem):
def generatePointer(self, contentFile):
"""The pointer content is the original content prefixed with "pointer-".
- The local filename of the large file storage is derived from the file content.
+ The local filename of the large file storage is derived from the
+ file content.
"""
with open(contentFile, 'r') as f:
content = next(f)
@@ -1452,8 +1465,8 @@ def generatePointer(self, contentFile):
return (gitMode, pointerContents, localLargeFile)
def pushFile(self, localLargeFile):
- """The remote filename of the large file storage is the same as the local
- one but in a different directory.
+ """The remote filename of the large file storage is the same as the
+ local one but in a different directory.
"""
remotePath = os.path.join(os.path.dirname(localLargeFile), '..', 'remote')
if not os.path.exists(remotePath):
@@ -1463,7 +1476,8 @@ def pushFile(self, localLargeFile):
class GitLFS(LargeFileSystem):
"""Git LFS as backend for the git-p4 large file system.
- See https://git-lfs.github.com/ for details."""
+ See https://git-lfs.github.com/ for details.
+ """
def __init__(self, *args):
LargeFileSystem.__init__(self, *args)
@@ -1729,20 +1743,20 @@ def check(self):
die("You have files opened with perforce! Close them before starting the sync.")
def separate_jobs_from_description(self, message):
- """Extract and return a possible Jobs field in the commit
- message. It goes into a separate section in the p4 change
- specification.
+ """Extract and return a possible Jobs field in the commit message. It
+ goes into a separate section in the p4 change specification.
- A jobs line starts with "Jobs:" and looks like a new field
- in a form. Values are white-space separated on the same
- line or on following lines that start with a tab.
+ A jobs line starts with "Jobs:" and looks like a new field in a
+ form. Values are white-space separated on the same line or on
+ following lines that start with a tab.
- This does not parse and extract the full git commit message
- like a p4 form. It just sees the Jobs: line as a marker
- to pass everything from then on directly into the p4 form,
- but outside the description section.
+ This does not parse and extract the full git commit message like a
+ p4 form. It just sees the Jobs: line as a marker to pass everything
+ from then on directly into the p4 form, but outside the description
+ section.
- Return a tuple (stripped log message, jobs string)."""
+ Return a tuple (stripped log message, jobs string).
+ """
m = re.search(r'^Jobs:', message, re.MULTILINE)
if m is None:
@@ -1753,9 +1767,10 @@ def separate_jobs_from_description(self, message):
return (stripped_message, jobtext)
def prepareLogMessage(self, template, message, jobs):
- """Edits the template returned from "p4 change -o" to insert
- the message in the Description field, and the jobs text in
- the Jobs field."""
+ """Edits the template returned from "p4 change -o" to insert the
+ message in the Description field, and the jobs text in the Jobs
+ field.
+ """
result = ""
inDescriptionSection = False
@@ -1880,11 +1895,13 @@ def canChangeChangelists(self):
def prepareSubmitTemplate(self, changelist=None):
"""Run "p4 change -o" to grab a change specification template.
+
This does not use "p4 -G", as it is nice to keep the submission
template in original order, since a human might edit it.
Remove lines in the Files section that show changes to files
- outside the depot path we're committing into."""
+ outside the depot path we're committing into.
+ """
[upstream, settings] = findUpstreamBranchPoint()
@@ -1947,8 +1964,10 @@ def prepareSubmitTemplate(self, changelist=None):
return template
def edit_template(self, template_file):
- """Invoke the editor to let the user change the submission
- message. Return true if okay to continue with the submit."""
+ """Invoke the editor to let the user change the submission message.
+
+ Return true if okay to continue with the submit.
+ """
# if configured to skip the editing part, just submit
if gitConfigBool("git-p4.skipSubmitEdit"):
@@ -2643,8 +2662,9 @@ def run(self, args):
class View(object):
- """Represent a p4 view ("p4 help views"), and map files in a
- repo according to the view."""
+ """Represent a p4 view ("p4 help views"), and map files in a repo according
+ to the view.
+ """
def __init__(self, client_name):
self.mappings = []
@@ -2653,9 +2673,10 @@ def __init__(self, client_name):
self.client_spec_path_cache = {}
def append(self, view_line):
- """Parse a view line, splitting it into depot and client
- sides. Append to self.mappings, preserving order. This
- is only needed for tag creation."""
+ """Parse a view line, splitting it into depot and client sides. Append
+ to self.mappings, preserving order. This is only needed for tag
+ creation.
+ """
# Split the view line into exactly two words. P4 enforces
# structure on these lines that simplifies this quite a bit.
@@ -2704,7 +2725,7 @@ def convert_client_path(self, clientFile):
return clientFile[len(self.client_prefix):]
def update_client_spec_path_cache(self, files):
- """ Caching file paths by "p4 where" batch query """
+ """Caching file paths by "p4 where" batch query."""
# List depot file paths exclude that already cached
fileArgs = [f['path'] for f in files if decode_path(f['path']) not in self.client_spec_path_cache]
@@ -2736,9 +2757,11 @@ def update_client_spec_path_cache(self, files):
self.client_spec_path_cache[depotFile] = b''
def map_in_client(self, depot_path):
- """Return the relative location in the client where this
- depot file should live. Returns "" if the file should
- not be mapped in the client."""
+ """Return the relative location in the client where this depot file
+ should live.
+
+ Returns "" if the file should not be mapped in the client.
+ """
if gitConfigBool("core.ignorecase"):
depot_path = depot_path.lower()
@@ -2889,10 +2912,10 @@ def extractJobsFromCommit(self, commit):
return jobs
def stripRepoPath(self, path, prefixes):
- """When streaming files, this is called to map a p4 depot path
- to where it should go in git. The prefixes are either
- self.depotPaths, or self.branchPrefixes in the case of
- branch detection."""
+ """When streaming files, this is called to map a p4 depot path to where
+ it should go in git. The prefixes are either self.depotPaths, or
+ self.branchPrefixes in the case of branch detection.
+ """
if self.useClientSpec:
# branch detection moves files up a level (the branch name)
@@ -2921,8 +2944,9 @@ def stripRepoPath(self, path, prefixes):
return path
def splitFilesIntoBranches(self, commit):
- """Look at each depotFile in the commit to figure out to what
- branch it belongs."""
+ """Look at each depotFile in the commit to figure out to what branch it
+ belongs.
+ """
if self.clientSpecDirs:
files = self.extractFilesFromCommit(commit)
@@ -3193,9 +3217,10 @@ def make_email(self, userid):
return "%s <a@b>" % userid
def streamTag(self, gitStream, labelName, labelDetails, commit, epoch):
- """ Stream a p4 tag.
- commit is either a git commit, or a fast-import mark, ":<p4commit>"
- """
+ """Stream a p4 tag.
+
+ Commit is either a git commit, or a fast-import mark, ":<p4commit>".
+ """
if verbose:
print("writing tag %s for commit %s" % (labelName, commit))
@@ -4246,8 +4271,7 @@ def __init__(self):
self.destbranch = "refs/remotes/p4-unshelved"
def renameBranch(self, branch_name):
- """ Rename the existing branch to branch_name.N
- """
+ """Rename the existing branch to branch_name.N ."""
found = True
for i in range(0,1000):
@@ -4263,9 +4287,9 @@ def renameBranch(self, branch_name):
sys.exit("gave up trying to rename existing branch {0}".format(sync.branch))
def findLastP4Revision(self, starting_point):
- """ Look back from starting_point for the first commit created by git-p4
- to find the P4 commit we are based on, and the depot-paths.
- """
+ """Look back from starting_point for the first commit created by git-p4
+ to find the P4 commit we are based on, and the depot-paths.
+ """
for parent in (range(65535)):
log = extractLogMessageFromGitCommit("{0}~{1}".format(starting_point, parent))
@@ -4276,8 +4300,9 @@ def findLastP4Revision(self, starting_point):
sys.exit("could not find git-p4 commits in {0}".format(self.origin))
def createShelveParent(self, change, branch_name, sync, origin):
- """ Create a commit matching the parent of the shelved changelist 'change'
- """
+ """Create a commit matching the parent of the shelved changelist
+ 'change'.
+ """
parent_description = p4_describe(change, shelved=True)
parent_description['desc'] = 'parent for shelved changelist {}\n'.format(change)
files = sync.extractFilesFromCommit(parent_description, shelved=False, shelved_cl=change)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 05/20] git-p4: convert descriptive class and function comments into docstrings
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (3 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 04/20] git-p4: improve consistency of docstring formatting Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 19:31 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 06/20] git-p4: remove commented code Joel Holdsworth
` (14 subsequent siblings)
19 siblings, 1 reply; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
Previously, a small number of functions, methods and classes were
documented using comments. This patch improves consistency by converting
these into docstrings similar to those that already exist in the script.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 162 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 92 insertions(+), 70 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 93f4ec843f..0bd1ba00ba 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -522,7 +522,7 @@ def p4_sync(f, *options):
def p4_add(f):
- # forcibly add file names with wildcards
+ """Forcibly add file names with wildcards."""
if wildcard_present(f):
p4_system(["add", "-f", f])
else:
@@ -590,12 +590,10 @@ def p4_describe(change, shelved=False):
return d
-#
-# Canonicalize the p4 type and return a tuple of the
-# base type, plus any modifiers. See "p4 help filetypes"
-# for a list and explanation.
-#
def split_p4_type(p4type):
+ """Canonicalize the p4 type and return a tuple of the base type, plus any
+ modifiers. See "p4 help filetypes" for a list and explanation.
+ """
p4_filetypes_historical = {
"ctempobj": "binary+Sw",
@@ -626,19 +624,18 @@ def split_p4_type(p4type):
return (base, mods)
-#
-# return the raw p4 type of a file (text, text+ko, etc)
-#
def p4_type(f):
+ """Return the raw p4 type of a file (text, text+ko, etc)."""
+
results = p4CmdList(["fstat", "-T", "headType", wildcard_encode(f)])
return results[0]['headType']
-#
-# Given a type base and modifier, return a regexp matching
-# the keywords that can be expanded in the file
-#
def p4_keywords_regexp_for_type(base, type_mods):
+ """Given a type base and modifier, return a regexp matching the keywords
+ that can be expanded in the file.
+ """
+
if base in ("text", "unicode", "binary"):
if "ko" in type_mods:
return re_ko_keywords
@@ -650,12 +647,11 @@ def p4_keywords_regexp_for_type(base, type_mods):
return None
-#
-# Given a file, return a regexp matching the possible
-# RCS keywords that will be expanded, or None for files
-# with kw expansion turned off.
-#
def p4_keywords_regexp_for_file(file):
+ """Given a file, return a regexp matching the possible RCS keywords that
+ will be expanded, or None for files with kw expansion turned off.
+ """
+
if not os.path.exists(file):
return None
else:
@@ -664,8 +660,9 @@ def p4_keywords_regexp_for_file(file):
def setP4ExecBit(file, mode):
- # Reopens an already open file and changes the execute bit to match
- # the execute bit setting in the passed in mode.
+ """Reopens an already open file and changes the execute bit to match the
+ execute bit setting in the passed in mode.
+ """
p4Type = "+x"
@@ -680,7 +677,7 @@ def setP4ExecBit(file, mode):
def getP4OpenedType(file):
- # Returns the perforce file type for the given file.
+ """Returns the perforce file type for the given file."""
result = p4_read_pipe(["opened", wildcard_encode(file)])
match = re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result)
@@ -690,8 +687,9 @@ def getP4OpenedType(file):
die("Could not determine file type for %s (result: '%s')" % (file, result))
-# Return the set of all p4 labels
def getP4Labels(depotPaths):
+ """Return the set of all p4 labels."""
+
labels = set()
if not isinstance(depotPaths, list):
depotPaths = [depotPaths]
@@ -703,8 +701,9 @@ def getP4Labels(depotPaths):
return labels
-# Return the set of all git tags
def getGitTags():
+ """Return the set of all git tags."""
+
gitTags = set()
for line in read_pipe_lines(["git", "tag"]):
tag = line.strip()
@@ -755,8 +754,9 @@ def parseDiffTreeEntry(entry):
def isModeExec(mode):
- # Returns True if the given git mode represents an executable file,
- # otherwise False.
+ """Returns True if the given git mode represents an executable file,
+ otherwise False.
+ """
return mode[-3:] == "755"
@@ -1274,13 +1274,14 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
def p4PathStartsWith(path, prefix):
- # This method tries to remedy a potential mixed-case issue:
- #
- # If UserA adds //depot/DirA/file1
- # and UserB adds //depot/dira/file2
- #
- # we may or may not have a problem. If you have core.ignorecase=true,
- # we treat DirA and dira as the same directory
+ """This method tries to remedy a potential mixed-case issue:
+
+ If UserA adds //depot/DirA/file1
+ and UserB adds //depot/dira/file2
+
+ we may or may not have a problem. If you have core.ignorecase=true,
+ we treat DirA and dira as the same directory.
+ """
if gitConfigBool("core.ignorecase"):
return path.lower().startswith(prefix.lower())
return path.startswith(prefix)
@@ -1332,13 +1333,15 @@ def getClientRoot():
return entry["Root"]
-#
-# P4 wildcards are not allowed in filenames. P4 complains
-# if you simply add them, but you can force it with "-f", in
-# which case it translates them into %xx encoding internally.
-#
def wildcard_decode(path):
- # Search for and fix just these four characters. Do % last so
+ """Decode P4 wildcards into %xx encoding
+
+ P4 wildcards are not allowed in filenames. P4 complains if you simply
+ add them, but you can force it with "-f", in which case it translates
+ them into %xx encoding internally.
+ """
+
+ # Search for and fix just these four characters. Do % last so
# that fixing it does not inadvertently create new %-escapes.
# Cannot have * in a filename in windows; untested as to
# what p4 would do in such a case.
@@ -1351,6 +1354,8 @@ def wildcard_decode(path):
def wildcard_encode(path):
+ """Encode %xx coded wildcards into P4 coding."""
+
# do % first to avoid double-encoding the %s introduced here
path = path.replace("%", "%25") \
.replace("*", "%2A") \
@@ -1597,7 +1602,7 @@ def p4UserId(self):
die("Could not find your p4 user id")
def p4UserIsMe(self, p4User):
- # return True if the given p4 user is actually me
+ """Return True if the given p4 user is actually me."""
me = self.p4UserId()
if not p4User or p4User != me:
return False
@@ -1800,7 +1805,9 @@ def prepareLogMessage(self, template, message, jobs):
return result
def patchRCSKeywords(self, file, regexp):
- # Attempt to zap the RCS keywords in a p4 controlled file matching the given regex
+ """Attempt to zap the RCS keywords in a p4 controlled file matching the
+ given regex.
+ """
(handle, outFileName) = tempfile.mkstemp(dir='.')
try:
with os.fdopen(handle, "wb") as outFile, open(file, "rb") as inFile:
@@ -1818,7 +1825,9 @@ def patchRCSKeywords(self, file, regexp):
print("Patched up RCS keywords in %s" % file)
def p4UserForCommit(self,id):
- # Return the tuple (perforce user,git email) for a given git commit id
+ """Return the tuple (perforce user,git email) for a given git commit
+ id.
+ """
self.getUserMapFromPerforceServer()
gitEmail = read_pipe(["git", "log", "--max-count=1",
"--format=%ae", id])
@@ -1829,7 +1838,7 @@ def p4UserForCommit(self,id):
return (self.emails[gitEmail],gitEmail)
def checkValidP4Users(self,commits):
- # check if any git authors cannot be mapped to p4 users
+ """Check if any git authors cannot be mapped to p4 users."""
for id in commits:
(user,email) = self.p4UserForCommit(id)
if not user:
@@ -1840,10 +1849,12 @@ def checkValidP4Users(self,commits):
die("Error: %s\nSet git-p4.allowMissingP4Users to true to allow this." % msg)
def lastP4Changelist(self):
- # Get back the last changelist number submitted in this client spec. This
- # then gets used to patch up the username in the change. If the same
- # client spec is being used by multiple processes then this might go
- # wrong.
+ """Get back the last changelist number submitted in this client spec.
+
+ This then gets used to patch up the username in the change. If the
+ same client spec is being used by multiple processes then this might
+ go wrong.
+ """
results = p4CmdList("client -o") # find the current client
client = None
for r in results:
@@ -1859,7 +1870,7 @@ def lastP4Changelist(self):
die("Could not get changelist number for last submit - cannot patch up user details")
def modifyChangelistUser(self, changelist, newUser):
- # fixup the user field of a changelist after it has been submitted.
+ """Fixup the user field of a changelist after it has been submitted."""
changes = p4CmdList("change -o %s" % changelist)
if len(changes) != 1:
die("Bad output from p4 change modifying %s to user %s" %
@@ -1882,8 +1893,9 @@ def modifyChangelistUser(self, changelist, newUser):
die("Could not modify user field of changelist %s to %s" % (changelist, newUser))
def canChangeChangelists(self):
- # check to see if we have p4 admin or super-user permissions, either of
- # which are required to modify changelists.
+ """Check to see if we have p4 admin or super-user permissions, either
+ of which are required to modify changelists.
+ """
results = p4CmdList(["protects", self.depotPath])
for r in results:
if 'perm' in r:
@@ -2334,9 +2346,11 @@ def applyCommit(self, id):
os.remove(fileName)
return submitted
- # Export git tags as p4 labels. Create a p4 label and then tag
- # with that.
def exportGitTags(self, gitTags):
+ """Export git tags as p4 labels. Create a p4 label and then tag with
+ that.
+ """
+
validLabelRegexp = gitConfig("git-p4.labelExportRegexp")
if len(validLabelRegexp) == 0:
validLabelRegexp = defaultLabelRegexp
@@ -2859,8 +2873,8 @@ def __init__(self):
self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
self.labels = {}
- # Force a checkpoint in fast-import and wait for it to finish
def checkpoint(self):
+ """Force a checkpoint in fast-import and wait for it to finish."""
self.gitStream.write("checkpoint\n\n")
self.gitStream.write("progress checkpoint\n\n")
self.gitStream.flush()
@@ -3006,9 +3020,9 @@ def encodeWithUTF8(self, path):
print('Path with non-ASCII characters detected. Used %s to encode: %s ' % (encoding, path))
return path
- # output one file from the P4 stream
- # - helper for streamP4Files
def streamOneP4File(self, file, contents):
+ """Output one file from the P4 stream - helper for streamP4Files."""
+
file_path = file['depotFile']
relPath = self.stripRepoPath(decode_path(file_path), self.branchPrefixes)
@@ -3100,8 +3114,8 @@ def streamOneP4Deletion(self, file):
if self.largeFileSystem and self.largeFileSystem.isLargeFile(relPath):
self.largeFileSystem.removeLargeFile(relPath)
- # handle another chunk of streaming data
def streamP4FilesCb(self, marshalled):
+ """Handle another chunk of streaming data."""
# catch p4 errors and complain
err = None
@@ -3165,8 +3179,9 @@ def streamP4FilesCb(self, marshalled):
self.stream_have_file_info = True
- # Stream directly from "p4 files" into "git fast-import"
def streamP4Files(self, files):
+ """Stream directly from "p4 files" into "git fast-import."""
+
filesForCommit = []
filesToRead = []
filesToDelete = []
@@ -3270,15 +3285,18 @@ def hasBranchPrefix(self, path):
return hasPrefix
def findShadowedFiles(self, files, change):
- # Perforce allows you commit files and directories with the same name,
- # so you could have files //depot/foo and //depot/foo/bar both checked
- # in. A p4 sync of a repository in this state fails. Deleting one of
- # the files recovers the repository.
- #
- # Git will not allow the broken state to exist and only the most recent
- # of the conflicting names is left in the repository. When one of the
- # conflicting files is deleted we need to re-add the other one to make
- # sure the git repository recovers in the same way as perforce.
+ """Perforce allows you commit files and directories with the same name,
+ so you could have files //depot/foo and //depot/foo/bar both checked
+ in. A p4 sync of a repository in this state fails. Deleting one of
+ the files recovers the repository.
+
+ Git will not allow the broken state to exist and only the most
+ recent of the conflicting names is left in the repository. When one
+ of the conflicting files is deleted we need to re-add the other one
+ to make sure the git repository recovers in the same way as
+ perforce.
+ """
+
deleted = [f for f in files if f['action'] in self.delete_actions]
to_check = set()
for f in deleted:
@@ -3395,8 +3413,11 @@ def commit(self, details, files, branch, parent = "", allow_empty=False):
print("Tag %s does not match with change %s: file count is different."
% (labelDetails["label"], change))
- # Build a dictionary of changelists and labels, for "detect-labels" option.
def getLabels(self):
+ """Build a dictionary of changelists and labels, for "detect-labels"
+ option.
+ """
+
self.labels = {}
l = p4CmdList(["labels"] + ["%s..." % p for p in self.depotPaths])
@@ -3422,11 +3443,12 @@ def getLabels(self):
if self.verbose:
print("Label changes: %s" % self.labels.keys())
- # Import p4 labels as git tags. A direct mapping does not
- # exist, so assume that if all the files are at the same revision
- # then we can use that, or it's something more complicated we should
- # just ignore.
def importP4Labels(self, stream, p4Labels):
+ """Import p4 labels as git tags. A direct mapping does not exist, so
+ assume that if all the files are at the same revision then we can
+ use that, or it's something more complicated we should just ignore.
+ """
+
if verbose:
print("import p4 labels: " + ' '.join(p4Labels))
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 06/20] git-p4: remove commented code
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (4 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 05/20] git-p4: convert descriptive class and function comments into docstrings Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 19:33 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 07/20] git-p4: sort and de-duplcate pylint disable list Joel Holdsworth
` (13 subsequent siblings)
19 siblings, 1 reply; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
Previously, the script contained commented code including Python 2 print
statements. Presumably, these were used as a developer aid at some point
in history. However, commented code and therefore this patch removes
them.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 7 -------
1 file changed, 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 0bd1ba00ba..ad385d6919 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3654,23 +3654,18 @@ def importNewBranch(self, branch, maxChange):
self.gitStream.flush()
branchPrefix = self.depotPaths[0] + branch + "/"
range = "@1,%s" % maxChange
- #print "prefix" + branchPrefix
changes = p4ChangesForPaths([branchPrefix], range, self.changes_block_size)
if len(changes) <= 0:
return False
firstChange = changes[0]
- #print "first change in branch: %s" % firstChange
sourceBranch = self.knownBranches[branch]
sourceDepotPath = self.depotPaths[0] + sourceBranch
sourceRef = self.gitRefForBranch(sourceBranch)
- #print "source " + sourceBranch
branchParentChange = int(p4Cmd(["changes", "-m", "1", "%s...@1,%s" % (sourceDepotPath, firstChange)])["change"])
- #print "branch parent: %s" % branchParentChange
gitParent = self.gitCommitByP4Change(sourceRef, branchParentChange)
if len(gitParent) > 0:
self.initialParents[self.gitRefForBranch(branch)] = gitParent
- #print "parent git commit: %s" % gitParent
self.importChanges(changes)
return True
@@ -3813,8 +3808,6 @@ def importHeadRevision(self, revision):
newestRevision = change
if info["action"] in self.delete_actions:
- # don't increase the file cnt, otherwise details["depotFile123"] will have gaps!
- #fileCnt = fileCnt + 1
continue
for prop in ["depotFile", "rev", "action", "type" ]:
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 07/20] git-p4: sort and de-duplcate pylint disable list
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (5 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 06/20] git-p4: remove commented code Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 08/20] git-p4: remove padding from lists, tuples and function arguments Joel Holdsworth
` (12 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
git-p4 contains configuration commands for pylint embedded in the header
comment.
Previously, these were combined onto single lines and not alphabetically
sorted. This patch breaks each disable command onto a separate line to
give cleaner diffs, removed duplicate entries, and sorts the list
alphabetically.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index ad385d6919..2f05ec10ca 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -7,13 +7,28 @@
# 2007 Trolltech ASA
# License: MIT <http://www.opensource.org/licenses/mit-license.php>
#
-# pylint: disable=invalid-name,missing-docstring,too-many-arguments,broad-except
-# pylint: disable=no-self-use,wrong-import-position,consider-iterating-dictionary
-# pylint: disable=wrong-import-order,unused-import,too-few-public-methods
-# pylint: disable=too-many-lines,ungrouped-imports,fixme,too-many-locals
-# pylint: disable=line-too-long,bad-whitespace,superfluous-parens
-# pylint: disable=too-many-statements,too-many-instance-attributes
-# pylint: disable=too-many-branches,too-many-nested-blocks
+# pylint: disable=bad-whitespace
+# pylint: disable=broad-except
+# pylint: disable=consider-iterating-dictionary
+# pylint: disable=disable
+# pylint: disable=fixme
+# pylint: disable=invalid-name
+# pylint: disable=line-too-long
+# pylint: disable=missing-docstring
+# pylint: disable=no-self-use
+# pylint: disable=superfluous-parens
+# pylint: disable=too-few-public-methods
+# pylint: disable=too-many-arguments
+# pylint: disable=too-many-branches
+# pylint: disable=too-many-instance-attributes
+# pylint: disable=too-many-lines
+# pylint: disable=too-many-locals
+# pylint: disable=too-many-nested-blocks
+# pylint: disable=too-many-statements
+# pylint: disable=ungrouped-imports
+# pylint: disable=unused-import
+# pylint: disable=wrong-import-order
+# pylint: disable=wrong-import-position
#
import sys
if sys.version_info.major < 3 and sys.version_info.minor < 7:
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 08/20] git-p4: remove padding from lists, tuples and function arguments
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (6 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 07/20] git-p4: sort and de-duplcate pylint disable list Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 09/20] git-p4: remove spaces around default arguments Joel Holdsworth
` (11 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 discourages use of extraneous padding inside any parenthesis,
brackets or braces in the "Pet Peeves" section:
https://www.python.org/dev/peps/pep-0008/#pet-peeves
This patch removes all cases of these.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 2f05ec10ca..39c1fb7d4c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1025,10 +1025,10 @@ def gitDeleteRef(ref):
def gitConfig(key, typeSpecifier=None):
if key not in _gitConfig:
- cmd = [ "git", "config" ]
+ cmd = ["git", "config"]
if typeSpecifier:
- cmd += [ typeSpecifier ]
- cmd += [ key ]
+ cmd += [typeSpecifier]
+ cmd += [key]
s = read_pipe(cmd, ignore_error=True)
_gitConfig[key] = s.strip()
return _gitConfig[key]
@@ -1047,7 +1047,7 @@ def gitConfigBool(key):
def gitConfigInt(key):
if key not in _gitConfig:
- cmd = [ "git", "config", "--int", key ]
+ cmd = ["git", "config", "--int", key]
s = read_pipe(cmd, ignore_error=True)
v = s.strip()
try:
@@ -1103,7 +1103,7 @@ def p4BranchesInGit(branchesAreInRemotes=True):
def branch_exists(branch):
"""Make sure that the given ref name really exists."""
- cmd = [ "git", "rev-parse", "--symbolic", "--verify", branch ]
+ cmd = ["git", "rev-parse", "--symbolic", "--verify", branch]
p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
out, _ = p.communicate()
out = decode_text_stream(out)
@@ -1319,7 +1319,7 @@ def getClientSpec():
client_name = entry["Client"]
# just the keys that start with "View"
- view_keys = [ k for k in entry.keys() if k.startswith("View") ]
+ view_keys = [k for k in entry.keys() if k.startswith("View")]
# hold this new View
view = View(client_name)
@@ -1585,8 +1585,8 @@ def processContent(self, git_mode, relPath, contents):
class Command:
- delete_actions = ( "delete", "move/delete", "purge" )
- add_actions = ( "add", "branch", "move/add" )
+ delete_actions = ("delete", "move/delete", "purge")
+ add_actions = ("add", "branch", "move/add")
def __init__(self):
self.usage = "usage: %prog [options]"
@@ -2593,7 +2593,7 @@ def run(self, args):
sys.exit(1)
except Exception as e:
print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nThe hook failed "\
- "with the error '{0}'".format(e.message) )
+ "with the error '{0}'".format(e.message))
sys.exit(1)
#
@@ -2798,7 +2798,7 @@ def map_in_client(self, depot_path):
if depot_path in self.client_spec_path_cache:
return self.client_spec_path_cache[depot_path]
- die( "Error: %s is not found in client spec path" % depot_path )
+ die("Error: %s is not found in client spec path" % depot_path)
return ""
@@ -3093,7 +3093,7 @@ def streamOneP4File(self, file, contents):
else:
if p4_version_string().find('/NT') >= 0:
text = text.replace(b'\r\n', b'\n')
- contents = [ text ]
+ contents = [text]
if type_base == "apple":
# Apple filetype files will be streamed as a concatenation of
@@ -3717,7 +3717,7 @@ def importChanges(self, changes, origin_revision=0):
for branch in branches.keys():
## HACK --hwn
branchPrefix = self.depotPaths[0] + branch + "/"
- self.branchPrefixes = [ branchPrefix ]
+ self.branchPrefixes = [branchPrefix]
parent = ""
@@ -3825,7 +3825,7 @@ def importHeadRevision(self, revision):
if info["action"] in self.delete_actions:
continue
- for prop in ["depotFile", "rev", "action", "type" ]:
+ for prop in ["depotFile", "rev", "action", "type"]:
details["%s%s" % (prop, fileCnt)] = info[prop]
fileCnt = fileCnt + 1
@@ -3978,7 +3978,7 @@ def run(self, args):
if branch_arg_given:
short = self.branch.split("/")[-1]
if short in branches:
- self.p4BranchesInGit = [ short ]
+ self.p4BranchesInGit = [short]
else:
self.p4BranchesInGit = branches.keys()
@@ -4259,7 +4259,7 @@ def run(self, args):
os.makedirs(self.cloneDestination)
chdir(self.cloneDestination)
- init_cmd = [ "git", "init" ]
+ init_cmd = ["git", "init"]
if self.cloneBare:
init_cmd.append("--bare")
retcode = subprocess.call(init_cmd)
@@ -4271,9 +4271,9 @@ def run(self, args):
# create a master branch and check out a work tree
if gitBranchExists(self.branch):
- system([ "git", "branch", currentGitBranch(), self.branch ])
+ system(["git", "branch", currentGitBranch(), self.branch])
if not self.cloneBare:
- system([ "git", "checkout", "-f" ])
+ system(["git", "checkout", "-f"])
else:
print('Not checking out any branch, use ' \
'"git checkout -q -b master <branch>"')
@@ -4404,7 +4404,7 @@ def run(self, args):
class P4Branches(Command):
def __init__(self):
Command.__init__(self)
- self.options = [ ]
+ self.options = []
self.description = ("Shows the git branches that hold imports and their "
+ "corresponding perforce depot paths")
self.verbose = False
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 09/20] git-p4: remove spaces around default arguments
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (7 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 08/20] git-p4: remove padding from lists, tuples and function arguments Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 10/20] git-p4: place a single space after every comma Joel Holdsworth
` (10 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that there should be no spaces around the = sign of
default argument values of functions.
This guideline is described here:
https://www.python.org/dev/peps/pep-0008/#other-recommendations
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 39c1fb7d4c..c6201b8d23 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1113,7 +1113,7 @@ def branch_exists(branch):
return out.rstrip() == branch
-def findUpstreamBranchPoint(head = "HEAD"):
+def findUpstreamBranchPoint(head="HEAD"):
branches = p4BranchesInGit()
# map from depot-path to branch name
branchByDepotPath = {}
@@ -1141,7 +1141,7 @@ def findUpstreamBranchPoint(head = "HEAD"):
return ["", settings]
-def createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/", silent=True):
+def createOrUpdateBranchesFromOrigin(localRefPrefix="refs/remotes/p4/", silent=True):
if not silent:
print("Creating/updating branch(es) in %s based on origin branch(es)"
% localRefPrefix)
@@ -2910,7 +2910,7 @@ def isPathWanted(self, path):
return True
return False
- def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):
+ def extractFilesFromCommit(self, commit, shelved=False, shelved_cl=0):
files = []
fnum = 0
while "depotFile%s" % fnum in commit:
@@ -3338,7 +3338,7 @@ def findShadowedFiles(self, files, change):
'rev': record['headRev'],
'type': record['headType']})
- def commit(self, details, files, branch, parent = "", allow_empty=False):
+ def commit(self, details, files, branch, parent="", allow_empty=False):
epoch = details["time"]
author = details["user"]
jobs = self.extractJobsFromCommit(details)
@@ -4487,8 +4487,8 @@ def main():
parser = optparse.OptionParser(cmd.usage.replace("%prog", "%prog " + cmdName),
options,
- description = cmd.description,
- formatter = HelpFormatter())
+ description=cmd.description,
+ formatter=HelpFormatter())
try:
(cmd, args) = parser.parse_args(sys.argv[2:], cmd)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 10/20] git-p4: place a single space after every comma
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (8 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 09/20] git-p4: remove spaces around default arguments Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 19:41 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 11/20] git-p4: remove extraneous spaces before function arguments Joel Holdsworth
` (9 subsequent siblings)
19 siblings, 1 reply; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
This patch improves consistency across git-p4 by ensuring all command
separated arguments to function invocations, tuples and lists are
separated by commas with a single space following.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index c6201b8d23..0e54be57b3 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -99,7 +99,7 @@ def p4_build_cmd(cmd):
user = gitConfig("git-p4.user")
if len(user) > 0:
- real_cmd += ["-u",user]
+ real_cmd += ["-u", user]
password = gitConfig("git-p4.password")
if len(password) > 0:
@@ -303,7 +303,7 @@ def run_hook_command(cmd, param):
cli = [cmd] + param
use_shell = False
if platform.system() == 'Windows':
- (root,ext) = os.path.splitext(cmd)
+ root, ext = os.path.splitext(cmd)
if ext == "":
exe_path = os.environ.get("EXEPATH")
if exe_path is None:
@@ -1839,7 +1839,7 @@ def patchRCSKeywords(self, file, regexp):
print("Patched up RCS keywords in %s" % file)
- def p4UserForCommit(self,id):
+ def p4UserForCommit(self, id):
"""Return the tuple (perforce user,git email) for a given git commit
id.
"""
@@ -1848,14 +1848,14 @@ def p4UserForCommit(self,id):
"--format=%ae", id])
gitEmail = gitEmail.strip()
if gitEmail not in self.emails:
- return (None,gitEmail)
+ return (None, gitEmail)
else:
- return (self.emails[gitEmail],gitEmail)
+ return (self.emails[gitEmail], gitEmail)
- def checkValidP4Users(self,commits):
+ def checkValidP4Users(self, commits):
"""Check if any git authors cannot be mapped to p4 users."""
for id in commits:
- (user,email) = self.p4UserForCommit(id)
+ user, email = self.p4UserForCommit(id)
if not user:
msg = "Cannot find p4 user for email %s in commit %s." % (email, id)
if gitConfigBool("git-p4.allowMissingP4Users"):
@@ -3272,7 +3272,7 @@ def streamTag(self, gitStream, labelName, labelDetails, commit, epoch):
gitStream.write("tagger %s\n" % tagger)
- print("labelDetails=",labelDetails)
+ print("labelDetails=", labelDetails)
if 'Description' in labelDetails:
description = labelDetails['Description']
else:
@@ -3478,7 +3478,7 @@ def importP4Labels(self, stream, p4Labels):
if not m.match(name):
if verbose:
- print("label %s does not match regexp %s" % (name,validLabelRegexp))
+ print("label %s does not match regexp %s" % (name, validLabelRegexp))
continue
if name in ignoredP4Labels:
@@ -3801,7 +3801,7 @@ def importHeadRevision(self, revision):
newestRevision = 0
fileCnt = 0
- fileArgs = ["%s...%s" % (p,revision) for p in self.depotPaths]
+ fileArgs = ["%s...%s" % (p, revision) for p in self.depotPaths]
for info in p4CmdList(["files"] + fileArgs):
@@ -4304,7 +4304,7 @@ def renameBranch(self, branch_name):
"""Rename the existing branch to branch_name.N ."""
found = True
- for i in range(0,1000):
+ for i in range(0, 1000):
backup_branch_name = "{0}.{1}".format(branch_name, i)
if not gitBranchExists(backup_branch_name):
gitUpdateRef(backup_branch_name, branch_name) # copy ref to backup
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 11/20] git-p4: remove extraneous spaces before function arguments
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (9 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 10/20] git-p4: place a single space after every comma Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 12/20] git-p4: remove redundant backslash-continuations inside brackets Joel Holdsworth
` (8 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that there should be no spaces before function arguments
in the in the "Pet Peeves" section:
https://www.python.org/dev/peps/pep-0008/#pet-peeves
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 0e54be57b3..f7b6df104f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -984,16 +984,16 @@ def extractSettingsGitLog(log):
values = {}
for line in log.split("\n"):
line = line.strip()
- m = re.search (r"^ *\[git-p4: (.*)\]$", line)
+ m = re.search(r"^ *\[git-p4: (.*)\]$", line)
if not m:
continue
- assignments = m.group(1).split (':')
+ assignments = m.group(1).split(':')
for a in assignments:
- vals = a.split ('=')
+ vals = a.split('=')
key = vals[0].strip()
- val = ('='.join (vals[1:])).strip()
- if val.endswith ('\"') and val.startswith('"'):
+ val = ('='.join(vals[1:])).strip()
+ if val.endswith('\"') and val.startswith('"'):
val = val[1:-1]
values[key] = val
@@ -2347,9 +2347,9 @@ def applyCommit(self, id):
# Revert changes if we skip this patch
if not submitted or self.shelve:
if self.shelve:
- print ("Reverting shelved files.")
+ print("Reverting shelved files.")
else:
- print ("Submission cancelled, undoing p4 changes.")
+ print("Submission cancelled, undoing p4 changes.")
sys.stdout.flush()
for f in editedFiles | filesToDelete:
p4_revert(f)
@@ -4018,7 +4018,7 @@ def run(self, args):
i = i - 1
break
- paths.append ("/".join(cur_list[:i + 1]))
+ paths.append("/".join(cur_list[:i + 1]))
self.previousDepotPaths = paths
@@ -4047,8 +4047,8 @@ def run(self, args):
else:
if self.depotPaths and self.depotPaths != args:
print("previous import used depot path %s and now %s was specified. "
- "This doesn't work!" % (' '.join (self.depotPaths),
- ' '.join (args)))
+ "This doesn't work!" % (' '.join(self.depotPaths),
+ ' '.join(args)))
sys.exit(1)
self.depotPaths = sorted(args)
@@ -4088,7 +4088,7 @@ def run(self, args):
if len(self.changesFile) == 0:
revision = "#head"
- p = re.sub ("\.\.\.$", "", p)
+ p = re.sub("\.\.\.$", "", p)
if not p.endswith("/"):
p += "/"
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 12/20] git-p4: remove redundant backslash-continuations inside brackets
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (10 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 11/20] git-p4: remove extraneous spaces before function arguments Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 13/20] git-p4: remove spaces between dictionary keys and colons Joel Holdsworth
` (7 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that backslash line continuations should only be used
for line-breaks outside parentheses.
This recommendation is described in the "Maximum Line Length" section:
https://www.python.org/dev/peps/pep-0008/#maximum-line-length
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index f7b6df104f..80101e02a1 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2273,13 +2273,13 @@ def applyCommit(self, id):
print(" " + self.clientPath)
print("")
print("To submit, use \"p4 submit\" to write a new description,")
- print("or \"p4 submit -i <%s\" to use the one prepared by" \
+ print("or \"p4 submit -i <%s\" to use the one prepared by"
" \"git p4\"." % fileName)
print("You can delete the file \"%s\" when finished." % fileName)
if self.preserveUser and p4User and not self.p4UserIsMe(p4User):
- print("To preserve change ownership by user %s, you must\n" \
- "do \"p4 change -f <change>\" after submitting and\n" \
+ print("To preserve change ownership by user %s, you must\n"
+ "do \"p4 change -f <change>\" after submitting and\n"
"edit the User field.")
if pureRenameCopy:
print("After submitting, renamed files must be re-synced.")
@@ -2422,7 +2422,7 @@ def exportGitTags(self, gitTags):
if self.dry_run:
print("Would create p4 label %s for tag" % name)
elif self.prepare_p4_only:
- print("Not creating p4 label %s for tag due to option" \
+ print("Not creating p4 label %s for tag due to option"
" --prepare-p4-only" % name)
else:
p4_write_pipe(["label", "-i"], labelTemplate)
@@ -2587,12 +2587,12 @@ def run(self, args):
if not self.no_verify:
try:
if not run_git_hook("p4-pre-submit"):
- print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nYou can skip " \
- "this pre-submission check by adding\nthe command line option '--no-verify', " \
+ print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nYou can skip "
+ "this pre-submission check by adding\nthe command line option '--no-verify', "
"however,\nthis will also skip the p4-changelist hook as well.")
sys.exit(1)
except Exception as e:
- print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nThe hook failed "\
+ print("\nThe p4-pre-submit hook failed, aborting the submit.\n\nThe hook failed "
"with the error '{0}'".format(e.message))
sys.exit(1)
@@ -2615,7 +2615,7 @@ def run(self, args):
applied.append(commit)
if self.prepare_p4_only:
if i < last:
- print("Processing only the first commit due to option" \
+ print("Processing only the first commit due to option"
" --prepare-p4-only")
break
else:
@@ -4275,7 +4275,7 @@ def run(self, args):
if not self.cloneBare:
system(["git", "checkout", "-f"])
else:
- print('Not checking out any branch, use ' \
+ print('Not checking out any branch, use '
'"git checkout -q -b master <branch>"')
# auto-set this variable if invoked with --use-client-spec
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 13/20] git-p4: remove spaces between dictionary keys and colons
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (11 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 12/20] git-p4: remove redundant backslash-continuations inside brackets Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 14/20] git-p4: ensure every comment has a single # Joel Holdsworth
` (6 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 makes no specific recommendation about spaces preceding colons in
dictionary declarations, but all the code examples contained with it
declare dictionaries with a single space after the colon, and none
before.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 80101e02a1..7bfa282f7e 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -4451,13 +4451,13 @@ def printUsage(commands):
commands = {
- "submit" : P4Submit,
- "commit" : P4Submit,
- "sync" : P4Sync,
- "rebase" : P4Rebase,
- "clone" : P4Clone,
- "branches" : P4Branches,
- "unshelve" : P4Unshelve,
+ "submit": P4Submit,
+ "commit": P4Submit,
+ "sync": P4Sync,
+ "rebase": P4Rebase,
+ "clone": P4Clone,
+ "branches": P4Branches,
+ "unshelve": P4Unshelve,
}
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 14/20] git-p4: ensure every comment has a single #
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (12 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 13/20] git-p4: remove spaces between dictionary keys and colons Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 15/20] git-p4: ensure there is a single space around all operators Joel Holdsworth
` (5 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that every comment should begin with a single '#'
character.
This guideline is described here:
https://www.python.org/dev/peps/pep-0008/#comments
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 7bfa282f7e..404de3e86d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -968,7 +968,7 @@ def branchExists(ref):
def extractLogMessageFromGitCommit(commit):
logMessage = ""
- ## fixme: title is first line of commit, not 1st paragraph.
+ # fixme: title is first line of commit, not 1st paragraph.
foundTitle = False
for log in read_pipe_lines(["git", "cat-file", "commit", commit]):
if not foundTitle:
@@ -3557,7 +3557,7 @@ def getBranchMapping(self):
continue
source = paths[0]
destination = paths[1]
- ## HACK
+ # HACK
if p4PathStartsWith(source, self.depotPaths[0]) and p4PathStartsWith(destination, self.depotPaths[0]):
source = source[len(self.depotPaths[0]):-4]
destination = destination[len(self.depotPaths[0]):-4]
@@ -3715,7 +3715,7 @@ def importChanges(self, changes, origin_revision=0):
if self.detectBranches:
branches = self.splitFilesIntoBranches(description)
for branch in branches.keys():
- ## HACK --hwn
+ # HACK --hwn
branchPrefix = self.depotPaths[0] + branch + "/"
self.branchPrefixes = [branchPrefix]
@@ -4105,7 +4105,7 @@ def run(self, args):
self.getLabels()
if self.detectBranches:
- ## FIXME - what's a P4 projectName ?
+ # FIXME - what's a P4 projectName ?
self.projectName = self.guessProjectName()
if self.hasOrigin:
@@ -4118,7 +4118,7 @@ def run(self, args):
for b in self.p4BranchesInGit:
if b != "master":
- ## FIXME
+ # FIXME
b = b[len(self.projectName):]
self.createdBranches.add(b)
@@ -4223,7 +4223,7 @@ def __init__(self):
self.cloneBare = False
def defaultDestination(self, args):
- ## TODO: use common prefix of args?
+ # TODO: use common prefix of args?
depotPath = args[0]
depotDir = re.sub("(@[^@]*)$", "", depotPath)
depotDir = re.sub("(#[^#]*)$", "", depotDir)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 15/20] git-p4: ensure there is a single space around all operators
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (13 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 14/20] git-p4: ensure every comment has a single # Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 16/20] git-p4: tidied visual indented lines of conditionals Joel Holdsworth
` (4 subsequent siblings)
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 requires that binary operators such as assignment and comparison
operators should always be surrounded by a pair of single spaces, and
recommends that all other binary operators should typically be surround
by single spaces.
The recommendation is given here in the "Other Recommendations"
section
https://www.python.org/dev/peps/pep-0008/#other-recommendations
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 404de3e86d..90e7922ebd 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -67,7 +67,7 @@
defaultLabelRegexp = r'[a-zA-Z0-9_\-.]+$'
# The block size is reduced automatically if required
-defaultBlockSize = 1<<20
+defaultBlockSize = 1 << 20
p4_access_checked = False
@@ -204,7 +204,7 @@ def prompt(prompt_text):
sys.stderr.flush()
sys.stdout.write(prompt_text)
sys.stdout.flush()
- response=sys.stdin.readline().strip().lower()
+ response = sys.stdin.readline().strip().lower()
if not response:
continue
response = response[0]
@@ -259,7 +259,7 @@ def run_git_hook(cmd, param=[]):
hooks_path = os.path.join(os.environ["GIT_DIR"], "hooks")
if not isinstance(param, list):
- param=[param]
+ param = [param]
# resolve hook file name, OS depdenent
hook_file = os.path.join(hooks_path, cmd)
@@ -2411,7 +2411,7 @@ def exportGitTags(self, gitTags):
# Create the label - use the same view as the client spec we are using
clientSpec = getClientSpec()
- labelTemplate = "Label: %s\n" % name
+ labelTemplate = "Label: %s\n" % name
labelTemplate += "Description:\n"
for b in body:
labelTemplate += "\t" + b + "\n"
@@ -2914,7 +2914,7 @@ def extractFilesFromCommit(self, commit, shelved=False, shelved_cl=0):
files = []
fnum = 0
while "depotFile%s" % fnum in commit:
- path = commit["depotFile%s" % fnum]
+ path = commit["depotFile%s" % fnum]
found = self.isPathWanted(decode_path(path))
if not found:
fnum = fnum + 1
@@ -3995,7 +3995,7 @@ def run(self, args):
p4Change = 0
for branch in self.p4BranchesInGit:
- logMsg = extractLogMessageFromGitCommit(self.refPrefix + branch)
+ logMsg = extractLogMessageFromGitCommit(self.refPrefix + branch)
settings = extractSettingsGitLog(logMsg)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 16/20] git-p4: tidied visual indented lines of conditionals
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (14 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 15/20] git-p4: ensure there is a single space around all operators Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 19:46 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 17/20] git-p4: compare to singletons with "is" and "is not" Joel Holdsworth
` (3 subsequent siblings)
19 siblings, 1 reply; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that when wrapping the arguments of conditional
statements, an extra level of indentation should be added to distinguish
arguments from the body of the statement.
This guideline is described here:
https://www.python.org/dev/peps/pep-0008/#indentation
This patch either adds the indentation, or removes unnecessary wrapping.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 90e7922ebd..0159e35adc 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1158,8 +1158,7 @@ def createOrUpdateBranchesFromOrigin(localRefPrefix="refs/remotes/p4/", silent=T
originHead = line
original = extractSettingsGitLog(extractLogMessageFromGitCommit(originHead))
- if ('depot-paths' not in original
- or 'change' not in original):
+ if 'depot-paths' not in original or 'change' not in original:
continue
update = False
@@ -2170,8 +2169,8 @@ def applyCommit(self, id):
if regexp:
# this file is a possibility...look for RCS keywords.
for line in read_pipe_lines(
- ["git", "diff", "%s^..%s" % (id, id), file],
- raw=True):
+ ["git", "diff", "%s^..%s" % (id, id), file],
+ raw=True):
if regexp.search(line):
if verbose:
print("got keyword match on %s in %s in %s" % (regex.pattern, line, file))
@@ -3181,9 +3180,9 @@ def streamP4FilesCb(self, marshalled):
self.stream_file[k] = marshalled[k]
if (verbose and
- 'streamContentSize' in self.stream_file and
- 'fileSize' in self.stream_file and
- 'depotFile' in self.stream_file):
+ 'streamContentSize' in self.stream_file and
+ 'fileSize' in self.stream_file and
+ 'depotFile' in self.stream_file):
size = int(self.stream_file["fileSize"])
if size > 0:
progress = 100*self.stream_file['streamContentSize']/size
@@ -4000,8 +3999,7 @@ def run(self, args):
settings = extractSettingsGitLog(logMsg)
self.readOptions(settings)
- if ('depot-paths' in settings
- and 'change' in settings):
+ if 'depot-paths' in settings and 'change' in settings:
change = int(settings['change']) + 1
p4Change = max(p4Change, change)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 17/20] git-p4: compare to singletons with "is" and "is not"
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (15 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 16/20] git-p4: tidied visual indented lines of conditionals Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 19:54 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 18/20] git-p4: only seperate code blocks by a single empty line Joel Holdsworth
` (2 subsequent siblings)
19 siblings, 1 reply; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that comparisons with singletons such as None should be
done with "is" and "is not", and never equality operators.
This guideline is described here:
https://www.python.org/dev/peps/pep-0008/#programming-recommendations
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 0159e35adc..2a3409ebd2 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -930,7 +930,7 @@ def p4Where(depotPath):
if data[:space] == depotPath:
output = entry
break
- if output == None:
+ if not output:
return ""
if output["code"] == "error":
return ""
@@ -952,7 +952,7 @@ def currentGitBranch():
def isValidGitDir(path):
- return git_dir(path) != None
+ return git_dir(path) is not None
def parseRevision(ref):
@@ -4497,7 +4497,7 @@ def main():
global verbose
verbose = cmd.verbose
if cmd.needsGit:
- if cmd.gitdir == None:
+ if not cmd.gitdir:
cmd.gitdir = os.path.abspath(".git")
if not isValidGitDir(cmd.gitdir):
# "rev-parse --git-dir" without arguments will try $PWD/.git
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 18/20] git-p4: only seperate code blocks by a single empty line
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (16 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 17/20] git-p4: compare to singletons with "is" and "is not" Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 19/20] git-p4: move inline comments to line above Joel Holdsworth
2022-01-12 13:46 ` [PATCH 20/20] git-p4: seperate multiple statements onto seperate lines Joel Holdsworth
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that blank lines should be used sparingly to separate
sections in the "Blank Lines" section:
https://www.python.org/dev/peps/pep-0008/#blank-lines
This patch replaces all double blank-line separations with a single
blank line.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 2 --
1 file changed, 2 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 2a3409ebd2..58f11ee41c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3593,7 +3593,6 @@ def getBranchMapping(self):
if source not in self.knownBranches:
lostAndFoundBranches.add(source)
-
for branch in lostAndFoundBranches:
self.knownBranches[branch] = branch
@@ -3816,7 +3815,6 @@ def importHeadRevision(self, revision):
sys.stderr.write("p4 exitcode: %s\n" % info['p4ExitCode'])
sys.exit(1)
-
change = int(info["change"])
if change > newestRevision:
newestRevision = change
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 19/20] git-p4: move inline comments to line above
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (17 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 18/20] git-p4: only seperate code blocks by a single empty line Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
2022-01-12 13:46 ` [PATCH 20/20] git-p4: seperate multiple statements onto seperate lines Joel Holdsworth
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 recommends that all inline comments should be separated from code
by two spaces, in the "Inline Comments" section:
https://www.python.org/dev/peps/pep-0008/#inline-comments
However, because all instances of these inline comments extended to an
excessive line length, they have been moved onto a seprate line.
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 58f11ee41c..679667611c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2052,7 +2052,9 @@ def get_diff_description(self, editedFiles, filesToAdd, symlinks):
for line in f.readlines():
newdiff += "+" + line
except UnicodeDecodeError:
- pass # Found non-text data and skip, since diff description should only include text
+ # Found non-text data and skip, since diff description
+ # should only include text
+ pass
f.close()
return (diff + newdiff).replace('\r\n', '\n')
@@ -3044,7 +3046,8 @@ def streamOneP4File(self, file, contents):
if 'fileSize' in self.stream_file:
size = int(self.stream_file['fileSize'])
else:
- size = 0 # deleted files don't get a fileSize apparently
+ # Deleted files don't get a fileSize apparently
+ size = 0
sys.stdout.write('\r%s --> %s (%s)\n' % (
file_path, relPath, format_size_human_readable(size)))
sys.stdout.flush()
@@ -4303,7 +4306,8 @@ def renameBranch(self, branch_name):
for i in range(0, 1000):
backup_branch_name = "{0}.{1}".format(branch_name, i)
if not gitBranchExists(backup_branch_name):
- gitUpdateRef(backup_branch_name, branch_name) # copy ref to backup
+ # Copy ref to backup
+ gitUpdateRef(backup_branch_name, branch_name)
gitDeleteRef(branch_name)
found = True
print("renamed old unshelve branch to {0}".format(backup_branch_name))
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 20/20] git-p4: seperate multiple statements onto seperate lines
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
` (18 preceding siblings ...)
2022-01-12 13:46 ` [PATCH 19/20] git-p4: move inline comments to line above Joel Holdsworth
@ 2022-01-12 13:46 ` Joel Holdsworth
19 siblings, 0 replies; 27+ messages in thread
From: Joel Holdsworth @ 2022-01-12 13:46 UTC (permalink / raw)
To: git, Luke Diamand, Junio C Hamano, Eric Sunshine
Cc: Tzadik Vanderhoof, Dorgon Chang, Joachim Kuebart, Daniel Levin,
Johannes Schindelin, Ben Keene, Andrew Oakley, Joel Holdsworth
PEP8 discourages the use of compound statements where there are multiple
statements on a single line in the "Other Recommendations" section:
https://www.python.org/dev/peps/pep-0008/#other-recommendations
Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
---
git-p4.py | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 679667611c..cd7ecb7aeb 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1264,7 +1264,8 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
else:
block_size = max(2, block_size // 2)
- if verbose: print("block size error, retrying with block size {0}".format(block_size))
+ if verbose:
+ print("block size error, retrying with block size {0}".format(block_size))
continue
except P4Exception as e:
die('Error retrieving changes description ({0})'.format(e.p4ExitCode))
@@ -1891,7 +1892,9 @@ def modifyChangelistUser(self, changelist, newUser):
(changelist, newUser))
c = changes[0]
- if c['User'] == newUser: return # nothing to do
+ if c['User'] == newUser:
+ # Nothing to do
+ return
c['User'] = newUser
# p4 does not understand format version 3 and above
input = marshal.dumps(c, 2)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 04/20] git-p4: improve consistency of docstring formatting
2022-01-12 13:46 ` [PATCH 04/20] git-p4: improve consistency of docstring formatting Joel Holdsworth
@ 2022-01-12 19:20 ` Eric Sunshine
0 siblings, 0 replies; 27+ messages in thread
From: Eric Sunshine @ 2022-01-12 19:20 UTC (permalink / raw)
To: Joel Holdsworth
Cc: Git List, Luke Diamand, Junio C Hamano, Tzadik Vanderhoof,
Dorgon Chang, Joachim Kuebart, Daniel Levin, Johannes Schindelin,
Ben Keene, Andrew Oakley
On Wed, Jan 12, 2022 at 8:47 AM Joel Holdsworth <jholdsworth@nvidia.com> wrote:
> This patch attempts to improve the consistency of the docstrings by
> making the following changes:
>
> - Rewraps all docstrings to a 79-character column limit.
> - Adds a full stop at the end of every docstring.
> - Removes any spaces after the opening triple-quotes of all
> docstrings.
> - Sets the hanging indent of multi-line docstrings to 3-spaces.
> - Ensures that the closing triple-quotes of multi-line docstrings are
> always on a new line indented by a 3-space indent.
A couple very minor comments below... probably not worth a re-roll.
> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
> ---
> diff --git a/git-p4.py b/git-p4.py
> @@ -271,20 +271,20 @@ def run_git_hook(cmd, param=[]):
> def run_hook_command(cmd, param):
> """Executes a git hook command
Missing full-stop (as described by the commit message)?
> - cmd = the command line file to be executed. This can be
> - a file that is run by OS association.
>
> - param = a list of parameters to pass to the cmd command
> + cmd -- the command line file to be executed. This can be a file that
> + is run by OS association.
> +
> + param -- a list of parameters to pass to the cmd command
These were changed from `=` to `--` (double hyphen), which is fine, but...
> @@ -715,21 +718,22 @@ def getGitTags():
> def parseDiffTreeEntry(entry):
> """Parses a single diff tree entry into its component elements.
>
> + See git-diff-tree(1) manpage for details about the format of the diff
> + output. This method returns a dictionary with the following elements:
> +
> + src_mode - The mode of the source file
> + dst_mode - The mode of the destination file
> + src_sha1 - The sha1 for the source file
> + dst_sha1 - The sha1 fr the destination file
> + status - The one letter status of the diff (i.e. 'A', 'M', 'D', etc)
> + status_score - The score for the status (applicable for 'C' and 'R'
> + statuses). This is None if there is no score.
> + src - The path for the source file.
> + dst - The path for the destination file. This is only present for
> + copy or renames. If it is not present, this is None.
... these just use `-` (single hyphen).
> + If the pattern is not matched, None is returned.
> + """
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 05/20] git-p4: convert descriptive class and function comments into docstrings
2022-01-12 13:46 ` [PATCH 05/20] git-p4: convert descriptive class and function comments into docstrings Joel Holdsworth
@ 2022-01-12 19:31 ` Eric Sunshine
0 siblings, 0 replies; 27+ messages in thread
From: Eric Sunshine @ 2022-01-12 19:31 UTC (permalink / raw)
To: Joel Holdsworth
Cc: Git List, Luke Diamand, Junio C Hamano, Tzadik Vanderhoof,
Dorgon Chang, Joachim Kuebart, Daniel Levin, Johannes Schindelin,
Ben Keene, Andrew Oakley
On Wed, Jan 12, 2022 at 8:47 AM Joel Holdsworth <jholdsworth@nvidia.com> wrote:
> Previously, a small number of functions, methods and classes were
> documented using comments. This patch improves consistency by converting
> these into docstrings similar to those that already exist in the script.
>
> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
> ---
> diff --git a/git-p4.py b/git-p4.py
> @@ -1332,13 +1333,15 @@ def getClientRoot():
> -# P4 wildcards are not allowed in filenames. P4 complains
> -# if you simply add them, but you can force it with "-f", in
> -# which case it translates them into %xx encoding internally.
> -#
> def wildcard_decode(path):
> - # Search for and fix just these four characters. Do % last so
> + """Decode P4 wildcards into %xx encoding
> +
> + P4 wildcards are not allowed in filenames. P4 complains if you simply
> + add them, but you can force it with "-f", in which case it translates
> + them into %xx encoding internally.
> + """
> +
> + # Search for and fix just these four characters. Do % last so
The unnecessary whitespace change in the "Search for and fix..." line
makes for a noisier diff and wasted a little bit of review time. Don't
know if it's worth a re-roll, though.
> @@ -3006,9 +3020,9 @@ def encodeWithUTF8(self, path):
> - # output one file from the P4 stream
> - # - helper for streamP4Files
> def streamOneP4File(self, file, contents):
> + """Output one file from the P4 stream - helper for streamP4Files."""
The hyphen is slightly difficult to interpret. A double hyphen, or
even better a semicolon, would have helped set off the second phrase
from the first. Alternatively, writing it as:
"""Output one file from the P4 stream.
This is a helper for streamP4Files().
"""
might be even clearer. Probably not worth a re-roll, though.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 06/20] git-p4: remove commented code
2022-01-12 13:46 ` [PATCH 06/20] git-p4: remove commented code Joel Holdsworth
@ 2022-01-12 19:33 ` Eric Sunshine
0 siblings, 0 replies; 27+ messages in thread
From: Eric Sunshine @ 2022-01-12 19:33 UTC (permalink / raw)
To: Joel Holdsworth
Cc: Git List, Luke Diamand, Junio C Hamano, Tzadik Vanderhoof,
Dorgon Chang, Joachim Kuebart, Daniel Levin, Johannes Schindelin,
Ben Keene, Andrew Oakley
On Wed, Jan 12, 2022 at 8:47 AM Joel Holdsworth <jholdsworth@nvidia.com> wrote:
> Previously, the script contained commented code including Python 2 print
> statements. Presumably, these were used as a developer aid at some point
> in history. However, commented code and therefore this patch removes
> them.
I'm having trouble interpreting the final sentence in this paragraph.
Did you mean "...commented code is useless, therefore remove it" or
something along those lines?
> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 10/20] git-p4: place a single space after every comma
2022-01-12 13:46 ` [PATCH 10/20] git-p4: place a single space after every comma Joel Holdsworth
@ 2022-01-12 19:41 ` Eric Sunshine
0 siblings, 0 replies; 27+ messages in thread
From: Eric Sunshine @ 2022-01-12 19:41 UTC (permalink / raw)
To: Joel Holdsworth
Cc: Git List, Luke Diamand, Junio C Hamano, Tzadik Vanderhoof,
Dorgon Chang, Joachim Kuebart, Daniel Levin, Johannes Schindelin,
Ben Keene, Andrew Oakley
On Wed, Jan 12, 2022 at 8:47 AM Joel Holdsworth <jholdsworth@nvidia.com> wrote:
> This patch improves consistency across git-p4 by ensuring all command
> separated arguments to function invocations, tuples and lists are
> separated by commas with a single space following.
>
> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
> ---
> diff --git a/git-p4.py b/git-p4.py
> @@ -303,7 +303,7 @@ def run_hook_command(cmd, param):
> - (root,ext) = os.path.splitext(cmd)
> + root, ext = os.path.splitext(cmd)
Someone who doesn't code in Python regularly might wonder if the
removal of parentheses on the left side of the assignment brings some
sort of semantic change with it. Perhaps the removal deserves mention
in the commit message?
Not necessarily worth a reroll.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 16/20] git-p4: tidied visual indented lines of conditionals
2022-01-12 13:46 ` [PATCH 16/20] git-p4: tidied visual indented lines of conditionals Joel Holdsworth
@ 2022-01-12 19:46 ` Eric Sunshine
0 siblings, 0 replies; 27+ messages in thread
From: Eric Sunshine @ 2022-01-12 19:46 UTC (permalink / raw)
To: Joel Holdsworth
Cc: Git List, Luke Diamand, Junio C Hamano, Tzadik Vanderhoof,
Dorgon Chang, Joachim Kuebart, Daniel Levin, Johannes Schindelin,
Ben Keene, Andrew Oakley
On Wed, Jan 12, 2022 at 8:47 AM Joel Holdsworth <jholdsworth@nvidia.com> wrote:
> git-p4: tidied visual indented lines of conditionals
Perhaps:
git-p4: normalize indentation of lines in conditionals
or something. (Probably not worth a re-roll.)
> PEP8 recommends that when wrapping the arguments of conditional
> statements, an extra level of indentation should be added to distinguish
> arguments from the body of the statement.
>
> This guideline is described here:
> https://www.python.org/dev/peps/pep-0008/#indentation
>
> This patch either adds the indentation, or removes unnecessary wrapping.
>
> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 17/20] git-p4: compare to singletons with "is" and "is not"
2022-01-12 13:46 ` [PATCH 17/20] git-p4: compare to singletons with "is" and "is not" Joel Holdsworth
@ 2022-01-12 19:54 ` Eric Sunshine
0 siblings, 0 replies; 27+ messages in thread
From: Eric Sunshine @ 2022-01-12 19:54 UTC (permalink / raw)
To: Joel Holdsworth
Cc: Git List, Luke Diamand, Junio C Hamano, Tzadik Vanderhoof,
Dorgon Chang, Joachim Kuebart, Daniel Levin, Johannes Schindelin,
Ben Keene, Andrew Oakley
On Wed, Jan 12, 2022 at 8:47 AM Joel Holdsworth <jholdsworth@nvidia.com> wrote:
> PEP8 recommends that comparisons with singletons such as None should be
> done with "is" and "is not", and never equality operators.
>
> This guideline is described here:
> https://www.python.org/dev/peps/pep-0008/#programming-recommendations
>
> Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com>
> ---
> diff --git a/git-p4.py b/git-p4.py
> @@ -930,7 +930,7 @@ def p4Where(depotPath):
> if data[:space] == depotPath:
> output = entry
> break
> - if output == None:
> + if not output:
> return ""
This changes semantics, doesn't it? It will now return "" for None,
False, and "". Is this change intentional? To match the intent of the
original, I would have expected it to be written:
if output is None:
return ""
but, having glanced at the larger context, I suppose the new semantics
may be okay(?).
> def parseRevision(ref):
> @@ -4497,7 +4497,7 @@ def main():
> if cmd.needsGit:
> - if cmd.gitdir == None:
> + if not cmd.gitdir:
> cmd.gitdir = os.path.abspath(".git")
Same question/observation as above. In order to save reviewers some
time, perhaps the commit message could say something about why the
semantic change is safe and/or desirable.
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-01-12 20:13 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12 13:46 [PATCH 00/20] git-p4: Various code tidy-ups Joel Holdsworth
2022-01-12 13:46 ` [PATCH 01/20] git-p4: add blank lines between functions and class definitions Joel Holdsworth
2022-01-12 13:46 ` [PATCH 02/20] git-p4: remove unneeded semicolons from statements Joel Holdsworth
2022-01-12 13:46 ` [PATCH 03/20] git-p4: indent with 4-spaces Joel Holdsworth
2022-01-12 13:46 ` [PATCH 04/20] git-p4: improve consistency of docstring formatting Joel Holdsworth
2022-01-12 19:20 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 05/20] git-p4: convert descriptive class and function comments into docstrings Joel Holdsworth
2022-01-12 19:31 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 06/20] git-p4: remove commented code Joel Holdsworth
2022-01-12 19:33 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 07/20] git-p4: sort and de-duplcate pylint disable list Joel Holdsworth
2022-01-12 13:46 ` [PATCH 08/20] git-p4: remove padding from lists, tuples and function arguments Joel Holdsworth
2022-01-12 13:46 ` [PATCH 09/20] git-p4: remove spaces around default arguments Joel Holdsworth
2022-01-12 13:46 ` [PATCH 10/20] git-p4: place a single space after every comma Joel Holdsworth
2022-01-12 19:41 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 11/20] git-p4: remove extraneous spaces before function arguments Joel Holdsworth
2022-01-12 13:46 ` [PATCH 12/20] git-p4: remove redundant backslash-continuations inside brackets Joel Holdsworth
2022-01-12 13:46 ` [PATCH 13/20] git-p4: remove spaces between dictionary keys and colons Joel Holdsworth
2022-01-12 13:46 ` [PATCH 14/20] git-p4: ensure every comment has a single # Joel Holdsworth
2022-01-12 13:46 ` [PATCH 15/20] git-p4: ensure there is a single space around all operators Joel Holdsworth
2022-01-12 13:46 ` [PATCH 16/20] git-p4: tidied visual indented lines of conditionals Joel Holdsworth
2022-01-12 19:46 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 17/20] git-p4: compare to singletons with "is" and "is not" Joel Holdsworth
2022-01-12 19:54 ` Eric Sunshine
2022-01-12 13:46 ` [PATCH 18/20] git-p4: only seperate code blocks by a single empty line Joel Holdsworth
2022-01-12 13:46 ` [PATCH 19/20] git-p4: move inline comments to line above Joel Holdsworth
2022-01-12 13:46 ` [PATCH 20/20] git-p4: seperate multiple statements onto seperate lines Joel Holdsworth
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).