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

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

chipx86
Review Board
release-4.0.x
reviewboard

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
[Demo] Enhance and fix up the setup experience in prepare-dev.py.
Fixes from review feedback.
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. Typo in description: prepare-dev.ppy -> prepare-dev.py

  3. Please flesh out the testing done section.

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

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

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

    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)
     
     

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

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

    Please add a blank line after return

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

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

  10. 
      
chipx86
Review request changed

Change Summary:

  • Fixed typos in docstrings and docs.
  • Added global ui before assigning ConsoleUI.

Description:

   

This modernizes the way we request input and display output on

~   prepare-dev.ppy, utilizing more of rb-site's ConsoleUI. This is a
  ~ 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.

Testing Done:

~  

Fixed the bug.

  ~

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

  + any errors.

Commits:

Summary
-
[Demo] Enhance and fix up the setup experience in prepare-dev.py.
+
[Demo] Enhance and fix up the setup experience in prepare-dev.py.
+
Fixes from review feedback.

Diff:

Revision 2 (+139 -49)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
david
  1. Ship It!
  2. 
      
Loading...