subprocess.run wrappers cause code duplication

Bug #1710253 reported by Robie Basak on 2017-08-11
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
usd-importer
Low
Nish Aravamudan

Bug Description

I suggest that we refactor how we call subprocess (wrapper for logging) functions to avoid duplication like this in git_repository.py:

+ kwargs = {}
+ kwargs['quiet'] = False
+ if verbose:
+ # If we are redirecting stdout/stderr to the console, we
+ # do not need to have run() also emit it
+ kwargs['quiet'] = True
+ kwargs['stdout'] = None
+ kwargs['stderr'] = None
+ self.git_run(['fetch', remote_name], **kwargs)

Nish Aravamudan (nacc) wrote :

Yeah, I think it makes sense for run() to take *different* parameters than subprocess, based upon what we actually need. In this case, we need quiet==True, so that run() doesn't output, but only because we want it to go to the console instead. So there should be a parameter like ouput_to_console=True/False, rather than having the caller know about that implementation detail.

Nish Aravamudan (nacc) wrote :

I started some of this refactoring in one of my branches, but it can be continued from there to incorporate this request.

Changed in usd-importer:
assignee: nobody → Nish Aravamudan (nacc)
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers