-
Notifications
You must be signed in to change notification settings - Fork 317
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
Fix Contributor Guide link #421
Conversation
@jhamrick I'm not entirely sure why tests are failing. It seems unrelated to my (innocuous) changes.
|
@vinaykola Not related to the tests failing...please use @jhamrick A couple of things:
Also, nedbat has a new release of coverage out. |
@willingc Updated to use |
Thanks @vinaykola ! Just a quick update that I will review this next week -- I'm currently rushing to get a paper submitted before the deadline, so won't have time to look at this right away. @willingc Yeah, I specifically picked the stable build for the default because that's what most people will be looking for if they have nbgrader installed from pip. There are currently issues with my RTD builds though that I'm aware of and am going to address as soon as I have time, hopefully next week... |
@jhamrick FYI. This looks good to merge. It's in the Contributing.md file in the root of the repo. I'll be prepping for Grace Hopper workshop and two panels this week. If I get a chance, I'll take a look at the build. Good luck on the paper 🌻 @vinaykola Your change looks good to me. Thanks for the contribution. 🐧 |
TMI but I'm betting this has much to do with the version of https://bitbucket.org/ned/coveragepy/issues/399/coverageexception-cant-combine-line-data where the issue were empty lines being generated by pytest-cov, https://bitbucket.org/ned/coveragepy/commits/74e2f7c366a2 allowing for https://travis-ci.org/jupyter/nbgrader/jobs/83507415#L458-L459 note I took one of the random failed jobs using Python 2.7 but each of the four failed tests failed in exactly the same manner,
|
@8leggedunicorn I would agree that you are on the correct path re: the errors. Nice documentation of the coverage changes. Do you want to look at the |
@willingc Sure I'll take a stab at it. |
@8leggedunicorn thanks for tracking that down! It looks like that issue was closed before coverage 4.0 proper was released, and it also looks 4.0 proper is what was installed (https://travis-ci.org/jupyter/nbgrader/jobs/83507415#L284), so it's odd that we're seeing that... but anyway, I will open a separate issue for that and we can keep trying to debug there. Thanks! @vinaykola this looks great, I am going to go ahead and merge even though the tests will now fail on master, but hopefully we can figure out the issue with coverage and fix that soon. Thanks! @willingc thanks for reviewing stuff and responding while I was away and super busy the last few weeks, I really appreciate it! |
@jhamrick No problem. Hope the paper went well. I'll be at Grace Hopper all next week doing a workshop and two panels. Send any cool students my way. There will be an open source booth. |
Fixes #420.