Merge lp:~kfogel/launchpad/google-analytics-everywhere into lp:launchpad

Proposed by Karl Fogel
Status: Merged
Approved by: Karl Fogel
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~kfogel/launchpad/google-analytics-everywhere
Merge into: lp:launchpad
Diff against target: 59 lines (+11/-2)
3 files modified
lib/lp/app/browser/tests/base-layout.txt (+9/-0)
lib/lp/app/templates/base-layout.pt (+1/-1)
lib/lp/bugs/stories/bugs/xx-bugtarget-bugs-page.txt (+1/-1)
To merge this branch: bzr merge lp:~kfogel/launchpad/google-analytics-everywhere
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Karl Fogel (community) Needs Resubmitting
Review via email: mp+22891@code.launchpad.net

Commit message

Activate Google Analytics for all of Launchpad, instead of just for 'edge'.

Description of the change

Activate Google Analytics for all of Launchpad, instead of just for 'edge'.

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) :
review: Approve
Revision history for this message
Karl Fogel (kfogel) wrote :

Gary, I had to make a couple of test changes (turns out that EC2 run was not a waste of time!). Can you please review them before I land? Thanks.

review: Needs Resubmitting
Revision history for this message
Karl Fogel (kfogel) wrote :

Passes EC2 now, by the way.

Revision history for this message
Gary Poster (gary) wrote :

gary_poster: kfogel: re https://code.edge.launchpad.net/~kfogel/launchpad/google-analytics-everywhere/+merge/22891 , it's fine. I would remove the lines with only ellipses, because I don't think they buy anything, but if you disagree, I'm ok with it as is.
[09:58am] kfogel: gary_poster: that's a good idea; I wasn't sure quite how "..." matching worked and actually meant to test without those lines, then forgot to.
[09:58am] gary_poster: kfogel: it's very greedy, think ``.*``
[09:59am] kfogel: gary_poster: ok
[09:59am] gary_poster: kfogel: fwiw actually, I think ``.+`` is more accurate
[09:59am] kfogel: gary_poster: gotcha

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/tests/base-layout.txt'
2--- lib/lp/app/browser/tests/base-layout.txt 2010-03-05 02:57:25 +0000
3+++ lib/lp/app/browser/tests/base-layout.txt 2010-04-07 14:07:35 +0000
4@@ -70,6 +70,9 @@
5 main_side
6 public
7 yui-skin-sam">
8+ <script type="text/javascript">
9+ ...google-analytics.com...
10+ </script>
11 <div class="yui-d0">
12 <BLANKLINE>
13 <div id="locationbar">
14@@ -274,6 +277,9 @@
15 searchless
16 public
17 yui-skin-sam">
18+ <script type="text/javascript">
19+ ...google-analytics.com...
20+ </script>
21 <div class="yui-d0">
22 ...
23
24@@ -386,6 +392,9 @@
25 locationless
26 public
27 yui-skin-sam">
28+ <script type="text/javascript">
29+ ...google-analytics.com...
30+ </script>
31 <div class="yui-d0">
32 ...
33
34
35=== modified file 'lib/lp/app/templates/base-layout.pt'
36--- lib/lp/app/templates/base-layout.pt 2010-02-04 11:17:39 +0000
37+++ lib/lp/app/templates/base-layout.pt 2010-04-07 14:07:35 +0000
38@@ -69,7 +69,7 @@
39 ${view/macro:pagetype}
40 ${view/context/fmt:public-private-css}
41 yui-skin-sam">
42- <script type="text/javascript" tal:condition="is_edge">
43+ <script type="text/javascript">
44 var _gaq = _gaq || [];
45 _gaq.push(['_setAccount', 'UA-12833497-1']);
46 _gaq.push(['_setDomainName', '.launchpad.net']);
47
48=== modified file 'lib/lp/bugs/stories/bugs/xx-bugtarget-bugs-page.txt'
49--- lib/lp/bugs/stories/bugs/xx-bugtarget-bugs-page.txt 2009-06-12 16:36:02 +0000
50+++ lib/lp/bugs/stories/bugs/xx-bugtarget-bugs-page.txt 2010-04-07 14:07:35 +0000
51@@ -12,7 +12,7 @@
52 so the search field is focused by default.
53
54 >>> from BeautifulSoup import BeautifulSoup
55- >>> print BeautifulSoup(browser.contents).body('script')[0]
56+ >>> print BeautifulSoup(browser.contents).body('script')[1]
57 <script...$('field.searchtext').focus();...
58
59 But once you're on the search results page, you're quite likely to want