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

added types to zerver/views/__init__.py #929

Closed
wants to merge 3 commits into from
Closed

Conversation

dhanus
Copy link
Contributor

@dhanus dhanus commented Jun 5, 2016

@smarx
Copy link

smarx commented Jun 5, 2016

Automated message from Dropbox CLA bot

@dhanus, thanks for the pull request! It looks like you haven't yet signed the Dropbox CLA. Please sign it here.

@smarx
Copy link

smarx commented Jun 5, 2016

Automated message from Dropbox CLA bot

@dhanus, thanks for signing the CLA!

@@ -354,13 +359,15 @@ def json_invite_users(request, user_profile, invitee_emails=REQ()):
return json_success()

def create_homepage_form(request, user_info=None):
# type: (HttpRequest) -> HomepageForm
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the user_info argument is missing here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@timabbott
Copy link
Sponsor Member

Thanks for doing this @dhanus! zerver/views/init.py is by a substantial margin our largest view file, so this is pretty awesome.

I posted some comments on annotations that I think aren't quite right (some of which likely explain the tests failing).

You can use tools/lint-all and tools/run-mypy to interactively debug the test failures shown by Travis CI in the checks, but I'd probably go through my comments first because I think the issues I flagged are why the tests are failing, so it might save you a bit of time to do it that way.

Let me know when you have a new version ready.

@timabbott
Copy link
Sponsor Member

Looks like you still have this mypy error: zerver/views/__init__.py:698: error: syntax error in type comment

@timabbott
Copy link
Sponsor Member

I'm going to take a pass at cleaning this up for merge, since I think some of the annotations in other files that this interacts with are wrong...

@timabbott
Copy link
Sponsor Member

OK, I got this passing and merged it as a261a6b. Thanks again for doing this @dhanus; I think this added like 3-4% to our total coverage!

@timabbott timabbott closed this Jun 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants