Code review comment for lp://staging/~jcsackett/loggerhead/add-privacy-ribbon

Revision history for this message
John A Meinel (jameinel) wrote :

My CSS and Javascript isn't particularly strong, however:
  display_privacy_notification()
calls
  setup_privacy_notification()

however this code:
189 +Y.on('domready', function() {
190 + var body = Y.get('body');
191 + if (body.hasClass('private')) {
192 + setup_privacy_notification();
193 + display_privacy_notification();
194 + }

Calls both directly, seems a bit odd.

Further, I did this change:
=== modified file 'loggerhead/apps/transport.py'
--- loggerhead/apps/transport.py 2011-03-16 14:43:36 +0000
+++ loggerhead/apps/transport.py 2011-11-03 13:22:01 +0000
@@ -63,7 +63,7 @@
             self.root.graph_cache,
             is_root=is_root,
             use_cdn=self._config.get_option('use_cdn'),
- )
+ private=True)
         return branch_app.app

     def app_for_non_branch(self, environ):

And tested locally with "bzr serve --http". I can see that the private attribute is set to True, but the pages themselves are *not* rendering a privacy bar.

Am I missing how you're supposed to be poking a branch to be considered "private"?

review: Needs Fixing

« Back to merge proposal