Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(202)

Issue 738: Started adapting old Browser code to work with GAE backend and XmlHttpRequests instead of JSONP

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by rob.ewaschuk
Modified:
2 months, 2 weeks ago
Reviewers:
fergald
SVN Base:
http://wto-informals.googlecode.com/svn/trunk/wto-informals/

Patch Set 1

Total comments: 4
Raw unified diffs Stats Side-by-side diffs with inline comments Delta from patch set
app.yaml 1 chunk 10 lines 0 comments
static/browser.css 2 chunks 21 lines 0 comments
static/browser.js 8 chunks 148 lines 0 comments
templates/browser.html 3 chunks 34 lines 0 comments
wto-informals.py 4 chunks 105 lines 4 comments

Messages

Total messages: 4
rob.ewaschuk
hey. uhh, this will be mostly greek to you, but take a look if you ...
2 months, 2 weeks ago
rob.ewaschuk
2 months, 2 weeks ago
rob.ewaschuk
2 months, 2 weeks ago
fergald
2 months, 2 weeks ago
not a whole lot to comment on

http://codereview.appspot.com/738/diff/1/5
File wto-informals.py (right):

http://codereview.appspot.com/738/diff/1/5#newcode18
Line 18: 
my first reaction was to say "make this an object". I still think it's right but
it doesn't buy you a whole lot.

I'm not sure about the temp_foo stuff. are you trying to make rebuilding the
cache atomic? If so, you still have a problem with having to set 2 variables at
the end.

Making it an object with all this going on inside it's __init__ would solve this
- you'd just say

members_cache = MembersCache()

and rebuilding it would just be reassigning a new instance to the global - which
would be atomic.

http://codereview.appspot.com/738/diff/1/5#newcode27
Line 27: memberships = Membership.all()
redundant variable

http://codereview.appspot.com/738/diff/1/5#newcode103
Line 103: def get(self, group):
what value does group get?

http://codereview.appspot.com/738/diff/1/5#newcode183
Line 183: #if not curUser:
I don't see any other user checking in the code. I've started playing with
appengine and almost the first thing I did was

class UserRequestHandler(webapp.RequestHandler):
  def get(self):
    user = users.get_current_user()
    if not user:	
      self.redirect(users.create_login_url(self.request.uri))
    else:
      self.request.user = user
      self.user_get()

or some-such
Sign in to reply to this message.

Powered by Google App Engine
This is Rietveld r168