Code review comment for lp:~elachuni/ubuntu-webcatalog/more-analytics

Revision history for this message
Michael Nelson (michael.nelson) wrote :

14:43 < achuni> noodles: if you have a while, could you have a look? https://code.launchpad.net/~elachuni/ubuntu-webcatalog/more-analytics/+merge/87758
14:44 < noodles> Sure thing
14:46 < noodles> achuni: is that a real ga-id on line 9? Does that mean our dev servers will be contributing to analytics? (not something you changed, but just worth asking :-)
14:47 < achuni> noodles: -36 is the staging id, so yep. Before, -24 meant that yes our dev server was contributing to analytics
14:48 < achuni> noodles: it should be ok to leave -36 in for dev, as we use the same id for all staging services nobody really takes those seriously
14:49 < noodles> achuni: sweet
14:50 < noodles> achuni: have you tested the bootstrap on lucid? (I can do so on my canonistack lucid instance, if not)
14:50 < achuni> noodles: not with the latest changes, nope, if you could that would be great
14:54 < noodles> achuni: bootstrapped fine on lucid with all tests passing.
14:54 < achuni> neat
14:54 < achuni> txs noodles
14:56 < noodles> achuni: the code+test look great - I assume it doesn't add significantly to the page load time? Might be worth a quick test using any web-dev tools. Anyway, approving.
14:57 < achuni> noodles: it doesn't add a noticeable wait to the page load, and we've got that code in place on sca/devportal already
14:57 < noodles> Great.

review: Approve

« Back to merge proposal