launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/standardize_js_934401 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #934401 in Launchpad itself: "longpoll and worlddata JS isn't in standard loc causing exceptions in build"
  https://bugs.launchpad.net/launchpad/+bug/934401

For more details, see:
https://code.launchpad.net/~rharding/launchpad/standardize_js_934401/+merge/93637

= Summary =
In building out JS build directory, there are two "exceptions" to the standard
pathing of the JS files, longpoll and worlddata. In order to allow for scripts
to work against our JS in a more standard way, we should move these under
lib/lp/app/javascript with the rest of the code in there. Precedent has been
set with things like inline help and such.

== Proposed fix ==
Move the tests into the app/javascript/tests directory, move the main files
into app/javascript.

== Pre-implementation notes ==
Had a brief pre-impl with Deryck to dbl check there weren't any recalled
historical reasons for these to be non-standard.

== Implementation details ==
Moved the files, update the paths in the longpoll tests since it's now
shallower in the tree.

== Tests ==
lib/lp/app/javascript/tests/test_longpoll.html

languages.js has no tests


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/tests/test_longpoll.html
  buildout-templates/bin/combo-rootdir.in
-- 
https://code.launchpad.net/~rharding/launchpad/standardize_js_934401/+merge/93637
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/standardize_js_934401 into lp:launchpad.

=== modified file 'buildout-templates/bin/combo-rootdir.in'
--- buildout-templates/bin/combo-rootdir.in	2012-02-15 14:16:45 +0000
+++ buildout-templates/bin/combo-rootdir.in	2012-02-17 19:08:19 +0000
@@ -32,10 +32,5 @@
 # ... and delete tests.
 find $BUILD_DIR/lp -name 'tests' -type d | xargs rm -rf
 
-# We have to move some modules around.
-rm $BUILD_DIR/lp/app/worlddata
-cp lib/lp/services/worlddata/javascript/languages.js $BUILD_DIR/lp
-cp lib/lp/app/longpoll/javascript/longpoll.js $BUILD_DIR/lp
-
 # Minify our JS.
 bin/py -m lp.scripts.utilities.js.jsmin_all $BUILD_DIR/lp

=== renamed file 'lib/lp/services/worlddata/javascript/languages.js' => 'lib/lp/app/javascript/languages.js'
=== renamed file 'lib/lp/app/longpoll/javascript/longpoll.js' => 'lib/lp/app/javascript/longpoll.js'
=== renamed file 'lib/lp/app/longpoll/javascript/tests/test_longpoll.html' => 'lib/lp/app/javascript/tests/test_longpoll.html'
--- lib/lp/app/longpoll/javascript/tests/test_longpoll.html	2012-02-10 13:51:47 +0000
+++ lib/lp/app/javascript/tests/test_longpoll.html	2012-02-17 19:08:19 +0000
@@ -11,23 +11,23 @@
 
       <!-- YUI and test setup -->
       <script type="text/javascript"
-              src="../../../../../../build/js/yui/yui/yui.js">
+              src="../../../../../build/js/yui/yui/yui.js">
       </script>
       <link rel="stylesheet"
-      href="../../../../../../build/js/yui/console/assets/console-core.css" />
-      <link rel="stylesheet"
-      href="../../../../../../build/js/yui/console/assets/skins/sam/console.css" />
-      <link rel="stylesheet"
-      href="../../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+      href="../../../../../build/js/yui/console/assets/console-core.css" />
+      <link rel="stylesheet"
+      href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+      <link rel="stylesheet"
+      href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
 
       <script type="text/javascript"
-              src="../../../../../../build/js/lp/app/testing/testrunner.js"></script>
+              src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
 
-      <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
+      <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
 
       <!-- Dependencies -->
       <script type="text/javascript"
-          src="../../../../../../build/js/lp/app/client.js"></script>
+          src="../../../../../build/js/lp/app/client.js"></script>
 
       <!-- The module under test. -->
       <script type="text/javascript" src="../longpoll.js"></script>

=== renamed file 'lib/lp/app/longpoll/javascript/tests/test_longpoll.js' => 'lib/lp/app/javascript/tests/test_longpoll.js'
=== removed directory 'lib/lp/app/longpoll/javascript'
=== removed directory 'lib/lp/app/longpoll/javascript/tests'
=== removed directory 'lib/lp/services/worlddata/javascript'