-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sourcery refactored master branch #1
base: master
Are you sure you want to change the base?
Conversation
if not d or len(d) == 0: | ||
return None | ||
else: | ||
return d | ||
return None if not d or len(d) == 0 else d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function git_dir
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
else: | ||
sys.stderr.write(msg + "\n") | ||
sys.exit(1) | ||
sys.stderr.write(msg + "\n") | ||
sys.exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function die
refactored with the following changes:
- Remove unnecessary else after guard condition (
remove-unnecessary-else
)
choices = set(m.group(1) for m in re.finditer(r"\[(.)\]", prompt_text)) | ||
choices = {m.group(1) for m in re.finditer(r"\[(.)\]", prompt_text)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function prompt
refactored with the following changes:
- Replace list(), dict() or set() with comprehension (
collection-builtin-to-comprehension
)
else: | ||
try: | ||
path.decode('ascii') | ||
except: | ||
path = path.decode(encoding, errors='replace') | ||
if verbose: | ||
print('Path with non-ASCII characters detected. Used {} to decode: {}'.format(encoding, path)) | ||
return path | ||
try: | ||
path.decode('ascii') | ||
except: | ||
path = path.decode(encoding, errors='replace') | ||
if verbose: | ||
print( | ||
f'Path with non-ASCII characters detected. Used {encoding} to decode: {path}' | ||
) | ||
return path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function decode_path
refactored with the following changes:
- Swap if/else branches [×2] (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
) - Replace call to format with f-string (
use-fstring-for-formatting
)
for p in param: | ||
args.append(p) | ||
args.extend(iter(param)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function run_git_hook
refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend
) - Simplify generator expression (
simplify-generator
)
else: | ||
type_base, type_mods = split_p4_type(p4_type(file)) | ||
return p4_keywords_regexp_for_type(type_base, type_mods) | ||
type_base, type_mods = split_p4_type(p4_type(file)) | ||
return p4_keywords_regexp_for_type(type_base, type_mods) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function p4_keywords_regexp_for_file
refactored with the following changes:
- Swap if/else branches [×2] (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
p4Type = p4Type[0:-1] | ||
p4Type = p4Type[:-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function setP4ExecBit
refactored with the following changes:
- Replace a[0:x] with a[:x] and a[x:len(a)] with a[x:] (
remove-redundant-slice-index
)
match = re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result) | ||
if match: | ||
return match.group(1) | ||
if match := re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result): | ||
return match[1] | ||
else: | ||
die("Could not determine file type for %s (result: '%s')" % (file, result)) | ||
die(f"Could not determine file type for {file} (result: '{result}')") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function getP4OpenedType
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups
)
for l in p4CmdList(["labels"] + ["%s..." % p for p in depotPaths]): | ||
for l in p4CmdList(["labels"] + [f"{p}..." for p in depotPaths]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function getP4Labels
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
match = _diff_tree_pattern.match(entry) | ||
if match: | ||
if match := _diff_tree_pattern.match(entry): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function parseDiffTreeEntry
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
if key in p4KeysContainingNonUtf8Chars() or \ | ||
key in p4KeysContainingBinaryData() or \ | ||
p4KeyContainsFilePaths(key): | ||
return False | ||
return True | ||
return ( | ||
key not in p4KeysContainingNonUtf8Chars() | ||
and key not in p4KeysContainingBinaryData() | ||
and not p4KeyContainsFilePaths(key) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function p4KeyWhichCanBeDirectlyDecoded
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Simplify boolean if expression (
boolean-if-exp-identity
) - Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast
)
sys.stderr.write("Opening pipe: {}\n".format(' '.join(cmd))) | ||
sys.stderr.write(f"Opening pipe: {' '.join(cmd)}\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function p4CmdList
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
) - Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
) - Simplify sequence length comparison (
simplify-len-comparison
) - Use named expression to simplify assignment and conditional (
use-named-expression
) - Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups
)
result.update(entry) | ||
result |= entry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function p4Cmd
refactored with the following changes:
- Merge dictionary updates via the union operator (
dict-assign-update-to-union
)
depotPathLong = depotPath + "..." | ||
depotPathLong = f"{depotPath}..." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function p4Where
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
assignments = m.group(1).split(':') | ||
assignments = m[1].split(':') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function extractSettingsGitLog
refactored with the following changes:
- Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups
)
assert False, "Method 'generatePointer' required in " + self.__class__.__name__ | ||
assert False, f"Method 'generatePointer' required in {self.__class__.__name__}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function LargeFileSystem.generatePointer
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
assert False, "Method 'pushFile' required in " + self.__class__.__name__ | ||
assert False, f"Method 'pushFile' required in {self.__class__.__name__}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function LargeFileSystem.pushFile
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
[relPath.endswith('.' + e) for e in gitConfigList('git-p4.largeFileExtensions')], | ||
False | ||
[ | ||
relPath.endswith(f'.{e}') | ||
for e in gitConfigList('git-p4.largeFileExtensions') | ||
], | ||
False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function LargeFileSystem.hasLargeFileExtension
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
pointerContents = 'pointer-' + content | ||
pointerContents = f'pointer-{content}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function MockLFS.generatePointer
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
['git', 'lfs', 'pointer', '--file=' + contentFile], | ||
stdout=subprocess.PIPE | ||
['git', 'lfs', 'pointer', f'--file={contentFile}'], | ||
stdout=subprocess.PIPE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function GitLFS.generatePointer
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups
)
if relPath == '.gitattributes': | ||
self.baseGitAttributes = contents | ||
return (git_mode, self.generateGitAttributes()) | ||
else: | ||
if relPath != '.gitattributes': | ||
return LargeFileSystem.processContent(self, git_mode, relPath, contents) | ||
self.baseGitAttributes = contents | ||
return (git_mode, self.generateGitAttributes()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function GitLFS.processContent
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
if not p4User or p4User != me: | ||
return False | ||
else: | ||
return True | ||
return bool(p4User and p4User == me) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function P4UserMap.p4UserIsMe
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
) - Simplify boolean if expression (
boolean-if-exp-identity
)
return home + "/.gitp4-usercache.txt" | ||
return f"{home}/.gitp4-usercache.txt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function P4UserMap.getUserCacheFilename
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
fulluser = fullname + " <" + email + ">" | ||
fulluser = f"{fullname} <{email}>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function P4UserMap.getUserMapFromPerforceServer
refactored with the following changes:
- Use f-string instead of string concatenation [×3] (
use-fstring-for-concatenation
)
cache = open(self.getUserCacheFilename(), 'rb') | ||
lines = cache.readlines() | ||
cache.close() | ||
with open(self.getUserCacheFilename(), 'rb') as cache: | ||
lines = cache.readlines() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function P4UserMap.loadUserMapFromCache
refactored with the following changes:
- Use
with
when opening file to ensure closure (ensure-file-closed
)
system(["sh", "-c", ('%s "$@"' % editor), editor, template_file]) | ||
system(["sh", "-c", f'{editor} "$@"', editor, template_file]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function P4Submit.edit_template
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
diff = "" | ||
for editedFile in editedFiles: | ||
diff += p4_read_pipe(['diff', '-du', | ||
wildcard_encode(editedFile)]) | ||
|
||
diff = "".join( | ||
p4_read_pipe(['diff', '-du', wildcard_encode(editedFile)]) | ||
for editedFile in editedFiles | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function P4Submit.get_diff_description
refactored with the following changes:
- Use str.join() instead of for loop (
use-join
) - Iterate over files directly rather than using readlines() (
use-file-iterator
) - Use
with
when opening file to ensure closure (ensure-file-closed
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
mark = dict() | ||
mark = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 38-75
refactored with the following changes:
- Replace
dict()
with{}
(dict-literal
) - Replace comparison with min/max call (
min-max-identity
) - Use x is None rather than x == None (
none-compare
) - Use f-string instead of string concatenation [×16] (
use-fstring-for-concatenation
) - Use items() to directly unpack dictionary values (
use-dict-items
) - Remove unnecessary call to keys() (
remove-dict-keys
)
env = '' | ||
elems = re.compile('(.*?)\s+<(.*)>').match(user) | ||
if elems: | ||
env += 'export GIT_AUTHOR_NAME="%s" ;' % elems.group(1) | ||
env += 'export GIT_COMMITTER_NAME="%s" ;' % elems.group(1) | ||
env += 'export GIT_AUTHOR_EMAIL="%s" ;' % elems.group(2) | ||
env += 'export GIT_COMMITTER_EMAIL="%s" ;' % elems.group(2) | ||
else: | ||
env += 'export GIT_AUTHOR_NAME="%s" ;' % user | ||
env += 'export GIT_COMMITTER_NAME="%s" ;' % user | ||
env += 'export GIT_AUTHOR_EMAIL= ;' | ||
env += 'export GIT_COMMITTER_EMAIL= ;' | ||
|
||
env += 'export GIT_AUTHOR_DATE="%s" ;' % date | ||
env += 'export GIT_COMMITTER_DATE="%s" ;' % date | ||
return env | ||
env = '' | ||
if elems := re.compile('(.*?)\s+<(.*)>').match(user): | ||
env += f'export GIT_AUTHOR_NAME="{elems[1]}" ;' | ||
env += f'export GIT_COMMITTER_NAME="{elems[1]}" ;' | ||
env += f'export GIT_AUTHOR_EMAIL="{elems[2]}" ;' | ||
env += f'export GIT_COMMITTER_EMAIL="{elems[2]}" ;' | ||
else: | ||
env += f'export GIT_AUTHOR_NAME="{user}" ;' | ||
env += f'export GIT_COMMITTER_NAME="{user}" ;' | ||
env += 'export GIT_AUTHOR_EMAIL= ;' | ||
env += 'export GIT_COMMITTER_EMAIL= ;' | ||
|
||
env += f'export GIT_AUTHOR_DATE="{date}" ;' | ||
env += f'export GIT_COMMITTER_DATE="{date}" ;' | ||
return env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function getgitenv
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Replace interpolated string formatting with f-string [×8] (
replace-interpolation-with-fstring
) - Replace m.group(x) with m[x] for re.Match objects [×4] (
use-getitem-for-re-match-groups
)
hgchildren[str(cset)] = () | ||
prnts = os.popen('hg log -r %d --template "{parents}"' % cset).read().strip().split(' ') | ||
prnts = map(lambda x: x[:x.find(':')], prnts) | ||
if prnts[0] != '': | ||
parent = prnts[0].strip() | ||
else: | ||
parent = str(cset - 1) | ||
hgchildren[parent] += ( str(cset), ) | ||
if len(prnts) > 1: | ||
mparent = prnts[1].strip() | ||
hgchildren[mparent] += ( str(cset), ) | ||
else: | ||
mparent = None | ||
|
||
hgparents[str(cset)] = (parent, mparent) | ||
|
||
if mparent: | ||
hgchildren[str(cset)] = () | ||
prnts = os.popen('hg log -r %d --template "{parents}"' % cset).read().strip().split(' ') | ||
prnts = map(lambda x: x[:x.find(':')], prnts) | ||
parent = prnts[0].strip() if prnts[0] != '' else str(cset - 1) | ||
hgchildren[parent] += ( str(cset), ) | ||
if len(prnts) > 1: | ||
mparent = prnts[1].strip() | ||
hgchildren[mparent] += ( str(cset), ) | ||
else: | ||
mparent = None | ||
|
||
hgparents[str(cset)] = (parent, mparent) | ||
|
||
if mparent: | ||
# For merge changesets, take either one, preferably the 'master' branch | ||
if hgbranch[mparent] == 'master': | ||
hgbranch[str(cset)] = 'master' | ||
else: | ||
hgbranch[str(cset)] = hgbranch[parent] | ||
else: | ||
# Normal changesets | ||
# For first children, take the parent branch, for the others create a new branch | ||
if hgchildren[parent][0] == str(cset): | ||
hgbranch[str(cset)] = hgbranch[parent] | ||
else: | ||
hgbranch[str(cset)] = "branch-" + str(cset) | ||
hgbranch[str(cset)] = ('master' if hgbranch[mparent] == 'master' else | ||
hgbranch[parent]) | ||
elif hgchildren[parent][0] == str(cset): | ||
hgbranch[str(cset)] = hgbranch[parent] | ||
else: | ||
hgbranch[str(cset)] = f"branch-{str(cset)}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 127-242
refactored with the following changes:
- Replace if statement with if expression [×2] (
assign-if-exp
) - Merge else clause's nested if statement into elif (
merge-else-if-into-elif
) - Use f-string instead of string concatenation [×5] (
use-fstring-for-concatenation
) - Replace interpolated string formatting with f-string [×6] (
replace-interpolation-with-fstring
)
This removes the following comments ( why? ):
# For first children, take the parent branch, for the others create a new branch
# Normal changesets
PR Summary
Benefits of this PR include greater efficiency, improved maintainability, and a more accurate version control history. |
Branch
master
refactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
master
branch, then run:Help us improve this pull request!