Always use pipeline compilers, add babel.

Review Request #7954 — Created Feb. 10, 2016 and submitted

Information

Review Board
release-2.6.x
46926cc...

Reviewers

This change switches us over to using pipeline's compilers all the time, not
just when PIPELINE_ENABLED = True, as well as adding the babel compiler to
the list. This means that compilation of LESS (and ES6!) happens on the
back-end instead of the front-end during development.

This is better than before because instead of recompiling all the stylesheets
on page load, we do it on-demand, and pipeline does mtime checks to verify
whether it needs to actually do any work. This significantly speeds up reloads
during development.

This adds new dev dependencies on lessc and babel. These are included in a
package.json file that allows one to just run "npm install" to get everything
necessary.

  • Verified that recompile on-demand from within the devserver worked as
    expected.
  • Converted common.js to common.es6 and made use of some ES6 features. Verified
    that what I got in the browser was transpiled ES5.
  • Converted a JS file in an extension to ES6 and saw similar results.
Description From Last Updated

'django_reset' imported but unused

reviewbotreviewbot

'from settings_local import *' used; unable to detect undefined names

reviewbotreviewbot

We don't use @param do we?

brenniebrennie

Missing semicolon.

ME medanat

`${$target.attr('href')}infobx/`

ME medanat

infobox

brenniebrennie

We use these flags for more than media building. They should stay.

chipx86chipx86

'django_reset' imported but unused

reviewbotreviewbot

'from settings_local import *' used; unable to detect undefined names

reviewbotreviewbot

'djblets' imported but unused

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

'from settings_local import *' used; unable to detect undefined names

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

'from settings_local import *' used; unable to detect undefined names

reviewbotreviewbot
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/settings.py
        reviewboard/test.py
        tests/runtests.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/static/rb/js/common.es6
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/.jshintrc
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/settings.py
        reviewboard/test.py
        tests/runtests.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/static/rb/js/common.es6
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/.jshintrc
    
    
  2. reviewboard/settings.py (Diff revision 1)
     
     
    Show all issues
     'django_reset' imported but unused
    
  3. reviewboard/settings.py (Diff revision 1)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  4. 
      
ME
  1. 
      
  2. reviewboard/static/rb/js/common.es6 (Diff revision 1)
     
     
    Is there a reason the semicolon was removed here?
  3. reviewboard/static/rb/js/common.es6 (Diff revision 1)
     
     
    This can be a const I believe.
  4. reviewboard/static/rb/js/common.es6 (Diff revision 1)
     
     
    `#${id}` instead of '#' + id maybe?
  5. 
      
brennie
  1. I will be so happy to write ES6 :)

  2. reviewboard/static/rb/js/common.es6 (Diff revision 1)
     
     
     
     
    Show all issues

    We don't use @param do we?

    1. No but I don't want to make this too invasive right now. common.js is pretty old and crufty stuff anyway and should eventually go away--I just wanted a proof of concept.

  3. 
      
ME
  1. 
      
  2. reviewboard/static/rb/js/common.es6 (Diff revision 1)
     
     
    Show all issues
    Missing semicolon.
  3. reviewboard/static/rb/js/common.es6 (Diff revision 1)
     
     
    Show all issues
    `${$target.attr('href')}infobx/`
  4. 
      
brennie
  1. 
      
  2. reviewboard/static/rb/js/common.es6 (Diff revision 1)
     
     
    Show all issues

    infobox

  3. 
      
chipx86
  1. Awesome. It'll be really nice to be able to have the smarter recompilation and support for ES6 :)

    We're discussing it in Slack already, but for posterity, the things I'm hoping we can change are:

    1) Using .js (or .es6.js) for the files instead, so everyone in the world (including Pygments) doesn't have to support .es6. Also, having a version in there makes it hard down the road for ES7, etc.
    2) I'd love to put built files in a separate directory, so we don't have a bunch of stuff all coming up when you grep. Something more manageable would be nice. Also, if we could do this, no need for .es6 at all.

    1. Well, the built files do end up going into reviewboard/htdocs/static (nothing writes to reviewboard/static anymore), but the problem is that pipeline uses a two-stage system where first they do a "collect" (which copies all the files into htdocs/static/), and then "compile" (which compiles things). We therefore can't have the same filename for input and output.

      I can implement a custom compiler subclass that does whatever path we want. I figure .es6.js is fine since I suspect the tooling will change before the next version of ES is codified.

  2. reviewboard/test.py (Diff revision 1)
     
     
     
    Show all issues

    We use these flags for more than media building. They should stay.

    1. These are now set earlier. runtests.py sets the environment variable, and settings.py sets the setting.

  3. 
      
david
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/settings.py
        reviewboard/test.py
        tests/runtests.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/js/.jshintrc
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/settings.py
        reviewboard/test.py
        tests/runtests.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/js/.jshintrc
    
    
  2. reviewboard/settings.py (Diff revision 2)
     
     
    Show all issues
     'django_reset' imported but unused
    
  3. reviewboard/settings.py (Diff revision 2)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  4. 
      
brennie
  1. Your testing done needs to be updated re: .es6.js vs .es6.

  2. 
      
chipx86
  1. Ship It!
  2. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        tests/runtests.py
        reviewboard/settings.py
        reviewboard/test.py
        contrib/internal/conf/settings_local.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/js/.jshintrc
        reviewboard/static/rb/css/pages/reviews.less
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        tests/runtests.py
        reviewboard/settings.py
        reviewboard/test.py
        contrib/internal/conf/settings_local.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/js/.jshintrc
        reviewboard/static/rb/css/pages/reviews.less
    
    
  2. contrib/internal/conf/settings_local.py (Diff revision 3)
     
     
    Show all issues
     'djblets' imported but unused
    
  3. reviewboard/settings.py (Diff revision 3)
     
     
    Show all issues
     'django_reset' imported but unused
    
  4. reviewboard/settings.py (Diff revision 3)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  5. 
      
david
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        tests/runtests.py
        reviewboard/settings.py
        reviewboard/test.py
        contrib/internal/conf/settings_local.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/js/.jshintrc
        reviewboard/static/rb/css/pages/reviews.less
    
    
    
    Tool: Pyflakes
    Processed Files:
        tests/runtests.py
        reviewboard/settings.py
        reviewboard/test.py
        contrib/internal/conf/settings_local.py
        reviewboard/staticbundles.py
    
    Ignored Files:
        package.json
        reviewboard/templates/base.html
        reviewboard/templates/js/pipeline.html
        .gitignore
        reviewboard/static/rb/js/common.es6.js
        reviewboard/static/rb/js/.jshintrc
        reviewboard/static/rb/css/pages/reviews.less
    
    
  2. reviewboard/settings.py (Diff revision 4)
     
     
    Show all issues
     'django_reset' imported but unused
    
  3. reviewboard/settings.py (Diff revision 4)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  4. 
      
brennie
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.6.x (cfefb40)