• 
      

    Fix up the formatting of the repository form JS

    Review Request #7761 — Created Nov. 9, 2015 and submitted

    Information

    Review Board
    release-2.5.x

    Reviewers

    This patch brings repositoryform.js into line with our style
    guidelines.

    Verified that the repository form still worked.
    Ran JSHint.

    Description From Last Updated

    This would be better written if we flip it: $('<p class="help" />') .text(text) .appendTo($('#row-' + field));

    david david

    Please undo the indentation change here (we want the ? and : to align within the parens)

    david david

    This can be: $hostingAccountPassRow.setVisible(hostingInfo.needs_authorization);

    david david

    This is awkward. Can we store $hostingAccount[0].selectedIndex] into a variable first? We should probably also call this $selectedOption since it's …

    david david

    setVisible()

    david david

    setVisible()

    david david

    Let's wrap $(this) once.

    david david

    Please undo this indentation change.

    david david

    This can use setVisible.

    david david

    setVisible

    david david

    Probably better as $hostingType.val()

    david david

    Probably better as $hostingType.val()

    david david

    Probably better as $bugTrackerType.val()

    david david
    reviewbot
    1. Tool: Pyflakes
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
      
      Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
    2. 
        
    brennie
    reviewbot
    1. Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
      
      Tool: Pyflakes
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
       
       
       
       
      Show all issues

      This would be better written if we flip it:

      $('<p class="help" />')
          .text(text)
          .appendTo($('#row-' + field));
      
    3. reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
       
       
       
       
      Show all issues

      Please undo the indentation change here (we want the ? and : to align within the parens)

    4. reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
       
       
       
       
       
       
      Show all issues

      This can be:

      $hostingAccountPassRow.setVisible(hostingInfo.needs_authorization);
      
    5. reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
       
       
       
      Show all issues

      This is awkward. Can we store $hostingAccount[0].selectedIndex] into a variable first?

      We should probably also call this $selectedOption since it's a jQuery

    6. reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
       
       
       
       
       
       
      Show all issues

      setVisible()

    7. reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
       
       
       
       
       
       
       
       
      Show all issues

      setVisible()

    8. 
        
    brennie
    reviewbot
    1. Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
      
      Tool: Pyflakes
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
    2. 
        
    david
    1. Found some more stuff. My word this code is old and crusty.

    2. Show all issues

      Let's wrap $(this) once.

    3. reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
       
       
       
      Show all issues

      Please undo this indentation change.

    4. reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
       
       
       
       
       
       
      Show all issues

      This can use setVisible.

    5. reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
       
       
       
       
       
       
      Show all issues

      setVisible

    6. Show all issues

      Probably better as $hostingType.val()

    7. Show all issues

      Probably better as $hostingType.val()

    8. Show all issues

      Probably better as $bugTrackerType.val()

    9. 
        
    brennie
    reviewbot
    1. Tool: Pyflakes
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
      
      Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/static/rb/js/repositoryform.js
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    brennie
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.0.x (45bd7b0)