Add new feature flag for the Unified Review Banner

Review Request #10187 — Created Sept. 27, 2018 and submitted

shoven
Review Board
release-4.0.x
62d5dae...
reviewboard, students

This is the first step of the larger project of creating the Unified
Review Banner Base Shell. Adding this feature flag will allow us to
toggle between the current banner experience and the new Unified Review
Banner experience to allow development on the Unified Banner to continue
without breaking the current experience.

Out of scope for this patch: using the feature flag to toggle between
the two experiences (will come in the next patch).

  • Ran all JS Unit tests (all pass)
  • Set the value in settings_local.py as
ENABLED_FEATURES = {
    'reviews.unified_banner': True/False
}

and tested output of RB.EnabledFeatures.unifiedBanner in
ReviewablePage.es6.js file (where the Unified Banner will be
instantiated)

  • 0
  • 0
  • 8
  • 0
  • 8
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

shoven
brennie
  1. Some small nits about your summary/testing done, otherwise this looks good 👍

  2. You can remove the bit about the failing tests, as that isn't relevant context to the patch.

  3. To do multi-line markdown code segments, the first line (with the triple backticks) needs to be blank

    ```
    like this
    ```

    Otherwise it interprets:
    ```foo
    bar
    ```

    as

    `` `foo bar` ``
    
  4. Can you wrap your summary & testing done at 72 characters?

  5. 
      
shoven
brennie
  1. Ship It!
  2. 
      
brennie
  1. 
      
  2. Tiny nit: summaries don't need a period.

  3. 
      
brennie
  1. 
      
  2. Also, your summary should be in the imperitive mood, i.e, Add instead of Adding.

  3. 
      
shoven
david
  1. One super tiny nit!

  2. reviewboard/reviews/features.py (Diff revision 2)
     
     

    This should be two blank lines instead of one.

  3. 
      
shoven
david
  1. Ship It!
  2. 
      
shoven
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (145097d)
Loading...