• 
      

    Update GitClient to use run_process().

    Review Request #12661 — Created Sept. 30, 2022 and submitted

    Information

    RBTools
    release-4.x

    Reviewers

    This switches GitClient to use run_process() instead of the legacy
    execute(). This ensures that every call site is deliberate in the
    kinds of data it's expected to work with and how errors are handled.

    The call sites have been audited to ensure that we're only redirecting
    stderr to stdout when we should and that we're explicitly handling error
    conditions. This may not be perfect, but it's already fixed up a few
    places where we may have been doing things wrong before.

    All git config calls are now handled by a _get_git_config(), which
    offers a standardized way of fetching configuration data, returning the
    string if found, None if not found, or raising SCMError if we get a
    fatal: result (such as if running outside of a Git tree).

    Unit tests pass.

    Posted this change for review.

    Summary ID
    Update GitClient to use run_process().
    This switches `GitClient` to use `run_process()` instead of the legacy `execute()`. This ensures that every call site is deliberate in the kinds of data it's expected to work with and how errors are handled. The call sites have been audited to ensure that we're only redirecting stderr to stdout when we should and that we're explicitly handling error conditions. This may not be perfect, but it's already fixed up a few places where we may have been doing things wrong before. All `git config` calls are now handled by a `_get_git_config()`, which offers a standardized way of fetching configuration data, returning the string if found, `None` if not found, or raising `SCMError` if we get a `fatal:` result (such as if running outside of a Git tree).
    27af259dd275c333d4210fee97a096eaf3df9f79
    Description From Last Updated

    'itertools' imported but unused Column: 1 Error code: F401

    reviewbotreviewbot

    Could change these unicodes to str.

    maubinmaubin

    Could change this unicode to str (and same for the rest of the args).

    maubinmaubin
    Checks run (1 failed, 1 succeeded)
    flake8 failed.
    JSHint passed.

    flake8

    maubin
    1. 
        
    2. rbtools/clients/git.py (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      Could change these unicodes to str.

      1. There's going to be a separate round of changes that will tackle types and docs.

    3. rbtools/clients/git.py (Diff revision 1)
       
       
      Show all issues

      Could change this unicode to str (and same for the rest of the args).

    4. 
        
    david
    1. Ship It!
    2. 
        
    maubin
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.x (e2842d1)