• 
      

    [Demo] Enhance and fix up the setup experience in prepare-dev.py.

    Review Request #11806 — Created Sept. 17, 2021 and discarded

    Information

    Review Board
    release-4.0.x

    Reviewers

    This modernizes the way we request input and display output on
    prepare-dev.py, utilizing more of rb-site's ConsoleUI. This is a
    better, more consistent way of displaying information and prompting for
    information.

    The text has been updated to be more user-friendly, to offer
    guidance for when things go wrong, and to provide next steps when the
    environment is prepared.

    Ran prepare-dev.py and verified that it set up my tree correctly without
    any errors.

    Summary ID
    [Demo] Enhance and fix up the setup experience in prepare-dev.py.
    This modernizes the way we request input and display output on `prepare-dev.ppy`, utilizing more of rb-site's `ConsoleUI`. This is a better, more consistent way of displaying information and prompting for information. The text has been updated to be more user-friendly, to offer guidance for when things go wrong, and to provide next steps when the environment is prepared.
    e30f2fcf4418c1fd419ba7d034fbcabb3accca92
    Fixes from review feedback.
    e77f748462f8a4a61f0dca6076dfd58b00a308da
    Description From Last Updated

    Typo in description: prepare-dev.ppy -> prepare-dev.py

    daviddavid

    Please flesh out the testing done section.

    daviddavid

    There needs to be a blank line between the description and "Type"

    daviddavid

    Typo: two spaces between "in" and "the"

    daviddavid

    E126 continuation line over-indented for hanging indent

    reviewbotreviewbot

    This will assign to a locally scoped ui variable. You need to add global ui

    daviddavid

    Please add a blank line after return

    daviddavid

    Because these get concatenated, you need a space at the end of the first string.

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

    flake8

    david
    1. Most of this looks good, but please pay close attention to the global variable issue.

    2. Show all issues

      Typo in description: prepare-dev.ppy -> prepare-dev.py

    3. Show all issues

      Please flesh out the testing done section.

    4. contrib/internal/prepare-dev.py (Diff revision 1)
       
       
       
      Show all issues

      There needs to be a blank line between the description and "Type"

    5. contrib/internal/prepare-dev.py (Diff revision 1)
       
       
      Show all issues

      Typo: two spaces between "in" and "the"

    6. contrib/internal/prepare-dev.py (Diff revision 1)
       
       

      What does os.system do?

      1. It's used to execute processes, similar to running a command in a shell. It's not always the right approach — the subprocess module is sometimes the better solution here.

    7. contrib/internal/prepare-dev.py (Diff revision 1)
       
       
      Show all issues

      This will assign to a locally scoped ui variable. You need to add global ui

    8. contrib/internal/prepare-dev.py (Diff revision 1)
       
       
       
      Show all issues

      Please add a blank line after return

    9. contrib/internal/prepare-dev.py (Diff revision 1)
       
       
       
      Show all issues

      Because these get concatenated, you need a space at the end of the first string.

    10. 
        
    chipx86
    david
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Discarded