kindof figured out how to test redirection
Whats being tested in //edit - Update hub with no data - Update hub with valid data - Update hub with invalid data
Misc - pep8 the file
with this we are at 60% coverage
Nice! Looks good to me. :thumbsup:
Does it work without this line here ^?
One comment, the rest looks good to me as well :)
Unfortunately no :( I could never get the the @app.before_request def check_auth(): too get called to set g.auth. Without this line it keeps complaining that g.auth.loggedin is not a variable or something like that. This was my kind of work around :/
@app.before_request def check_auth():
I can take a look again at http://flask.pocoo.org/docs/0.11/api/#flask.Flask.before_request
to get that working instead of the way im loading it
So maybe we should adjust the code to check if there is an auth attribute to flask.g
auth
flask.g
Maybe something like https://pagure.io/pagure/blob/master/f/pagure/init.py#_156 ?
i like that a lot :) let me see if I can get that implemented
rebased
pep8 files and added a new function to see if flask.g.auth is set
This seems wrong, I think authenticated() should cover all cases, no?
authenticated()
These pep8 changes seem odd to me but that might just be me :)
Pingou i made the authenticated function do all the checking.
also are we strict on pep8 80 chars? i really don't like that limit 100 would be nice
also are we strict on pep8 80 chars?
I kinda like it :)
I only ask about the pep8 char limit since this kind of looks funny split. But i'll follow the convention of 80 chars
What about simplifying it to : return hasattr(flask.g, 'auth') and flask.g.auth is not None and flask.g.auth.logged_in
return hasattr(flask.g, 'auth') and flask.g.auth is not None and flask.g.auth.logged_in
But on these occasion I don't think we should apply pep8, at all :)
what if the session has the login stored? I could shorten up this function but I feel like we should still call check_auth() if this returns false hasattr(flask.g, 'auth') and flask.g.auth is not None and flask.g.auth.logged_in
hasattr(flask.g, 'auth') and flask.g.auth is not None and flask.g.auth.logged_in
It should be called automatically, otherwise I'd consider if it's not a bug in flask or in our usage of it
Okay :) there's a few more spaces i'd like to exempt the 80 char limit ill note them as a comment.
Considering that this is an URL, temporary and that the part behind the hash sign is not a comment per say, I'd just ignore pep8 here :)
But here we could follow the 80 chars rule and split at the and to make a condition per line :)
and
LGTM. :ship: it
Pull-Request has been merged by skrzepto
kindof figured out how to test redirection
Whats being tested in //edit
- Update hub with no data
- Update hub with valid data
- Update hub with invalid data
Misc
- pep8 the file