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

Add type hints #11

Closed
zupo opened this issue Jul 25, 2018 · 9 comments
Closed

Add type hints #11

zupo opened this issue Jul 25, 2018 · 9 comments

Comments

@zupo
Copy link
Member

zupo commented Jul 25, 2018

What the title says. They must be checked on Travis on every run.

Bonus karma points to calculate types coverage and fail Travis if under a certain number. We have this in woocart-app already: https://github.com/niteoweb/woocart-app/blob/master/Makefile#L80

@mandarvaze
Copy link
Contributor

Link throws 404
I could not find "woocart-app" in the list of repos either.

If it is a private repo - this issue needs to be removed from "otasks"

@zupo
Copy link
Member Author

zupo commented May 23, 2019

@mandarvaze
Copy link
Contributor

OK, I'll take a look at this.

@mandarvaze
Copy link
Contributor

Here are the updates so far :

  1. I "borrowed" type_coverage.py script from the above link
  2. There is no setup.cfg here (like pyramind_openapi3) - so I used --linecount-report typecov option to run mypy
  3. I made a trivial change to pyramid_heroku/migrate.py without which mypy won't even continue.
  4. I was able to run the type_coverage.py (It failed due to just 15% coverage)
  5. Looking at .travis.yml, I think if I add a line at the end, it should be sufficient.

@zupo

  1. Is it OK to copy the type_coverage.py ? Linking just a single file from another git repo is non-trivial.
  2. Should I also "copy" setup.cfg from pyramid_openapi3? Or is it OK to begin with command line option --linecount-report typecov ? (We can bring in setup.cfg later)
  3. Do I need to run travis locally before I submit a PR ?

@zupo
Copy link
Member Author

zupo commented May 28, 2019

Is it OK to copy the type_coverage.py?

Yes.

Or is it OK to begin with command line option --linecount-report typecov?

Yes.

Do I need to run travis locally before I submit a PR ?

No.

mandarvaze pushed a commit to mandarvaze/pyramid_heroku that referenced this issue May 28, 2019
mandarvaze added a commit to mandarvaze/pyramid_heroku that referenced this issue May 28, 2019
@mandarvaze
Copy link
Contributor

@zupo I may have misunderstood the scope of this issue.

Is making type coverage 100% part of the scope of this issue ?
Or
just getting the infrastructure in place for checking the type coverage ?

Currently, the coverage is at 15%

I could change the min_coverage to 15, so that travis run passes for this PR.
Making the type_coverage 100% could be a separate task.

@zupo
Copy link
Member Author

zupo commented May 28, 2019

... fail Travis if under a certain number.

15 is good. And yes, getting coverage up to 100% is definitely a separate PR.

@mandarvaze
Copy link
Contributor

Please see PR #28 (Finally travis passed)

@mandarvaze
Copy link
Contributor

@zupo Now that the PR is merged, should this issue be closed ?

@zupo zupo closed this as completed May 30, 2019
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

No branches or pull requests

2 participants