Merge lp:~rockstar/launchpad/code-js-file-reorg into lp:launchpad

Proposed by Paul Hummer
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~rockstar/launchpad/code-js-file-reorg
Merge into: lp:launchpad
Prerequisite: lp:~rockstar/launchpad/code-js-reorg
Diff against target: 244 lines (+44/-45)
10 files modified
lib/canonical/launchpad/javascript/code/branch.bugspeclinks.js (+2/-2)
lib/canonical/launchpad/javascript/code/branch.status.js (+3/-4)
lib/canonical/launchpad/javascript/code/branchmergeproposal.diff.js (+3/-3)
lib/canonical/launchpad/javascript/code/branchmergeproposal.reviewcomment.js (+3/-3)
lib/canonical/launchpad/javascript/code/branchmergeproposal.status.js (+2/-2)
lib/lp/app/templates/base-layout-macros.pt (+17/-4)
lib/lp/bugs/templates/bugtask-index.pt (+2/-2)
lib/lp/code/templates/branch-index.pt (+4/-18)
lib/lp/code/templates/branch-related-bugs-specs.pt (+2/-2)
lib/lp/code/templates/branchmergeproposal-index.pt (+6/-5)
To merge this branch: bzr merge lp:~rockstar/launchpad/code-js-file-reorg
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+21085@code.launchpad.net

Description of the change

In the early days of javascript, no sheriff meant no law. No law meant crime
was at an all time high.

This branch basically removes some of our javascript crimes. Specifically,
when we created javascript, we named the file and namespace whatever seemed to
work. When everyone has their own ideas on how this works, it means we have a
great amount of inconsistency. I changed this by naming the javascript files
to be <model>.<attribute>.js in the code javascript. This also meant that I
needed to change some of the YUI namespaces.

All the windmill tests pass, etc.

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Looks suitably boring. Is the code going to move into lib/lp/code at some point?

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== renamed file 'lib/canonical/launchpad/javascript/code/branchlinks.js' => 'lib/canonical/launchpad/javascript/code/branch.bugspeclinks.js'
2--- lib/canonical/launchpad/javascript/code/branchlinks.js 2010-03-11 23:28:28 +0000
3+++ lib/canonical/launchpad/javascript/code/branch.bugspeclinks.js 2010-03-11 23:28:30 +0000
4@@ -7,9 +7,9 @@
5 * @requires base, lazr.anim, lazr.formoverlay
6 */
7
8-YUI.add('lp.code.branchlinks', function(Y) {
9+YUI.add('lp.code.branch.bugspeclinks', function(Y) {
10
11-var namespace = Y.namespace('lp.code.branchlinks');
12+var namespace = Y.namespace('lp.code.branch.bugspeclinks');
13
14 var lp_client; // The LP client
15
16
17=== renamed file 'lib/canonical/launchpad/javascript/code/branchstatus.js' => 'lib/canonical/launchpad/javascript/code/branch.status.js'
18--- lib/canonical/launchpad/javascript/code/branchstatus.js 2010-03-11 23:28:28 +0000
19+++ lib/canonical/launchpad/javascript/code/branch.status.js 2010-03-11 23:28:30 +0000
20@@ -7,9 +7,9 @@
21 * @requires node, lazr.choiceedit, lp.client.plugins
22 */
23
24-YUI.add('lp.code.branchstatus', function(Y) {
25+YUI.add('lp.code.branch.status', function(Y) {
26
27-var namespace = Y.namespace('lp.code.branchstatus');
28+var namespace = Y.namespace('lp.code.branch.status');
29
30 /*
31 * Connect the branch status to the javascript events.
32@@ -18,8 +18,7 @@
33
34 var status_content = Y.one('#branch-details-status-value');
35
36- if
37- (conf.user_can_edit_status) {
38+ if (conf.user_can_edit_status) {
39 var status_choice_edit = new Y.ChoiceSource({
40 contentBox: status_content,
41 value: conf.status_value,
42
43=== renamed file 'lib/canonical/launchpad/javascript/code/subscription.js' => 'lib/canonical/launchpad/javascript/code/branch.subscription.js'
44=== renamed file 'lib/canonical/launchpad/javascript/code/popupdiff.js' => 'lib/canonical/launchpad/javascript/code/branchmergeproposal.diff.js'
45--- lib/canonical/launchpad/javascript/code/popupdiff.js 2010-03-11 23:28:28 +0000
46+++ lib/canonical/launchpad/javascript/code/branchmergeproposal.diff.js 2010-03-11 23:28:30 +0000
47@@ -3,14 +3,14 @@
48 *
49 * Code for handling the popup diffs in the pretty overlays.
50 *
51- * @module lp.code.branchmergeproposal.popupdiff
52+ * @module lp.code.branchmergeproposal.diff
53 * @requires node
54 */
55
56-YUI.add('lp.code.branchmergeproposal.popupdiff', function(Y) {
57+YUI.add('lp.code.branchmergeproposal.diff', function(Y) {
58
59 // Grab the namespace in order to be able to expose the connect method.
60-var namespace = Y.namespace('lp.code.branchmergeproposal.popupdiff');
61+var namespace = Y.namespace('lp.code.branchmergeproposal.diff');
62
63 // The launchpad js client used.
64 var lp_client;
65
66=== renamed file 'lib/canonical/launchpad/javascript/code/codereview.js' => 'lib/canonical/launchpad/javascript/code/branchmergeproposal.reviewcomment.js'
67--- lib/canonical/launchpad/javascript/code/codereview.js 2010-03-11 23:28:28 +0000
68+++ lib/canonical/launchpad/javascript/code/branchmergeproposal.reviewcomment.js 2010-03-11 23:28:30 +0000
69@@ -3,13 +3,13 @@
70 *
71 * Library for code review javascript.
72 *
73- * @module lp.code.codereview
74+ * @module lp.code.branchmergeproposal.reviewcomment
75 * @requires base, lazr.anim, lazr.formoverlay
76 */
77
78-YUI.add('lp.code.codereview', function(Y) {
79+YUI.add('lp.code.branchmergeproposal.reviewcomment', function(Y) {
80
81-var namespace = Y.namespace('lp.code.codereview');
82+var namespace = Y.namespace('lp.code.branchmergeproposal.reviewcomment');
83
84 var reviewer_picker; // The "Request a review" overlay
85 var lp_client;
86
87=== renamed file 'lib/canonical/launchpad/javascript/code/branchmergeproposal.js' => 'lib/canonical/launchpad/javascript/code/branchmergeproposal.status.js'
88--- lib/canonical/launchpad/javascript/code/branchmergeproposal.js 2010-03-11 23:28:28 +0000
89+++ lib/canonical/launchpad/javascript/code/branchmergeproposal.status.js 2010-03-11 23:28:30 +0000
90@@ -6,9 +6,9 @@
91 * @requires node, lazr.choiceedit, lp.client.plugins
92 */
93
94-YUI.add('lp.code.branchmergeproposal', function(Y) {
95+YUI.add('lp.code.branchmergeproposal.status', function(Y) {
96
97-var namespace = Y.namespace('lp.code.branchmergeproposal');
98+var namespace = Y.namespace('lp.code.branchmergeproposal.status');
99
100 /*
101 * Connect the branch status to the javascript events.
102
103=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
104--- lib/lp/app/templates/base-layout-macros.pt 2010-03-09 18:41:20 +0000
105+++ lib/lp/app/templates/base-layout-macros.pt 2010-03-11 23:28:30 +0000
106@@ -187,10 +187,23 @@
107 tal:attributes="src string:${lp_js}/client/client.js"></script>
108
109 <script type="text/javascript"
110- tal:attributes="src string:${lp_js}/code/branchmergeproposal.js">
111- </script>
112- <script type="text/javascript"
113- tal:attributes="src string:${lp_js}/code/codereview.js"></script>
114+ tal:attributes="src string:${lp_js}/code/branch.bugspeclinks.js">
115+ </script>
116+ <script type="text/javascript"
117+ tal:attributes="src string:${lp_js}/code/branch.status.js">
118+ </script>
119+ <script type="text/javascript"
120+ tal:attributes="src string:${lp_js}/code/branchmergeproposal.diff.js">
121+ </script>
122+ <script type="text/javascript"
123+ tal:attributes="src string:${lp_js}/code/branch.subscription.js">
124+ </script>
125+ <script type="text/javascript"
126+ tal:attributes="src string:${lp_js}/code/branchmergeproposal.status.js">
127+ </script>
128+ <script type="text/javascript"
129+ tal:attributes="src string:${lp_js}/code/branchmergeproposal.reviewcomment.js"></script>
130+
131 <script type="text/javascript"
132 tal:attributes="src string:${lp_js}/lp/comment.js"></script>
133 <script type="text/javascript"
134
135=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
136--- lib/lp/bugs/templates/bugtask-index.pt 2010-03-11 23:28:28 +0000
137+++ lib/lp/bugs/templates/bugtask-index.pt 2010-03-11 23:28:30 +0000
138@@ -38,10 +38,10 @@
139 </tal:devmode>
140 <script type="text/javascript">
141 LPS.use('base', 'node', 'oop', 'event', 'bugs.bugtask_index',
142- 'lp.code.branchmergeproposal.popupdiff', function(Y) {
143+ 'lp.code.branchmergeproposal.diff', function(Y) {
144 Y.bugs.setup_bugtask_index();
145 Y.on('load', function(e) {
146- Y.lp.code.branchmergeproposal.popupdiff.connect_diff_links();
147+ Y.lp.code.branchmergeproposal.diff.connect_diff_links();
148 }, window);
149 });
150 </script>
151
152=== modified file 'lib/lp/code/templates/branch-index.pt'
153--- lib/lp/code/templates/branch-index.pt 2010-03-11 23:28:28 +0000
154+++ lib/lp/code/templates/branch-index.pt 2010-03-11 23:28:30 +0000
155@@ -27,25 +27,11 @@
156 }
157
158 </style>
159- <tal:devmode condition="devmode">
160- <script type="text/javascript"
161- tal:attributes="src string:${lp_js}/code/branchlinks.js">
162- </script>
163- <script type="text/javascript"
164- tal:attributes="src string:${lp_js}/code/branchstatus.js">
165- </script>
166- <script type="text/javascript"
167- tal:attributes="src string:${lp_js}/code/popupdiff.js">
168- </script>
169- <script type="text/javascript"
170- tal:attributes="src string:${lp_js}/code/subscription.js">
171- </script>
172- </tal:devmode>
173 <script type="text/javascript"
174 tal:content="string:
175 LPS.use('node', 'event', 'widget', 'plugin', 'overlay',
176- 'lazr.choiceedit', 'lp.code.branchstatus',
177- 'lp.code.branchmergeproposal.popupdiff',
178+ 'lazr.choiceedit', 'lp.code.branch.status',
179+ 'lp.code.branchmergeproposal.diff',
180 'lp.code.branch.subscription', function(Y) {
181
182 Y.on('load', function(e) {
183@@ -54,8 +40,8 @@
184 });
185 subscription_portlet.render();
186
187- Y.lp.code.branchstatus.connect_status(${view/status_config});
188- Y.lp.code.branchmergeproposal.popupdiff.connect_diff_links();
189+ Y.lp.code.branch.status.connect_status(${view/status_config});
190+ Y.lp.code.branchmergeproposal.diff.connect_diff_links();
191 },
192 window);
193 });
194
195=== modified file 'lib/lp/code/templates/branch-related-bugs-specs.pt'
196--- lib/lp/code/templates/branch-related-bugs-specs.pt 2010-03-11 23:28:28 +0000
197+++ lib/lp/code/templates/branch-related-bugs-specs.pt 2010-03-11 23:28:30 +0000
198@@ -42,14 +42,14 @@
199 string:&lt;script id='branchlink-script' type='text/javascript'&gt;" />
200 <!--
201
202- LPS.use('io-base', 'lp.code.branchlinks', function(Y) {
203+ LPS.use('io-base', 'lp.code.branch.bugspeclinks', function(Y) {
204
205 if(Y.UA.ie) {
206 return;
207 }
208
209 Y.on('domready', function() {
210- Y.lp.code.branchlinks.connect_branchlinks();
211+ Y.lp.code.branch.bugspeclinks.connect_branchlinks();
212 });
213
214 });
215
216=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
217--- lib/lp/code/templates/branchmergeproposal-index.pt 2010-03-11 23:28:28 +0000
218+++ lib/lp/code/templates/branchmergeproposal-index.pt 2010-03-11 23:28:30 +0000
219@@ -228,8 +228,8 @@
220 string:&lt;script id='codereview-script' type='text/javascript'&gt;" />
221 conf = <tal:status-config replace="view/status_config" />
222 <!--
223- LPS.use('io-base', 'lp.code.codereview', 'lp.code.branchmergeproposal',
224- 'lp.comment', function(Y) {
225+ LPS.use('io-base', 'lp.code.branchmergeproposal.reviewcomment',
226+ 'lp.code.branchmergeproposal.status', 'lp.comment', function(Y) {
227
228 Y.on('load', function() {
229 var logged_in = LP.client.links['me'] !== undefined;
230@@ -242,10 +242,11 @@
231 return;
232 }
233
234- Y.lp.code.branchmergeproposal.connect_status(conf);
235+ Y.lp.code.branchmergeproposal.status.connect_status(conf);
236 }
237- Y.lp.code.codereview.connect_links();
238- (new Y.lp.code.codereview.NumberToggle()).render();
239+ Y.lp.code.branchmergeproposal.reviewcomment.connect_links();
240+ (new Y.lp.code.branchmergeproposal.reviewcomment.NumberToggle()
241+ ).render();
242 }, window);
243 });
244 -->