Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate URLs with float, fix bug in apistar injector #74

Merged
merged 2 commits into from Jul 11, 2019

Conversation

@amatissart
Copy link
Member

commented Jul 10, 2019

@amatissart amatissart requested a review from GuillaumeGomez Jul 10, 2019

)
return self.injector.run(funcs, state)
except Exception as exc:
# This is the only change with the parent class.

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Jul 10, 2019

Contributor

Instead of keeping everything inside this exception block, since we return from there, why not remove one indent level and put pass and some logging into the exception block instead?

This comment has been minimized.

Copy link
@amatissart

amatissart Jul 10, 2019

Author Member

I prefer to stick to the parent class implementation if you don't mind.
And exc is used in the block, so it looks more explicit this way.

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Jul 10, 2019

Contributor

I really don't like the fact that you have inner exception blocks in it. What about a function call instead?

This comment has been minimized.

Copy link
@amatissart

amatissart Jul 11, 2019

Author Member

it should look cleaner now

@GuillaumeGomez

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

Some code looking improvements to do but otherwise looks good. :)

@amatissart amatissart force-pushed the fix-injector branch from 89ab835 to bba3d3d Jul 10, 2019

@amatissart amatissart force-pushed the fix-injector branch from bba3d3d to bdac81d Jul 11, 2019

@amatissart amatissart requested a review from GuillaumeGomez Jul 11, 2019

@amatissart amatissart merged commit 9197cea into master Jul 11, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@amatissart amatissart deleted the fix-injector branch Aug 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.