-
Notifications
You must be signed in to change notification settings - Fork 57
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 failing tests #165
fix failing tests #165
Conversation
closes #156 |
Thanks, Jebej! |
Also, if you would be so kind, in file 'src/Logger/websocketlogger.jl', line 9: |
I would have never figured that out! |
this one seems to be failing on 0.7 |
the new import is causing a warning on 0.7, any idea on which version to start adding it? |
c8cbfea
to
88f34e6
Compare
88f34e6
to
788ef2b
Compare
okay I think that's enough here, more tests are failing on nightly but they can be fixed in an other PR |
We're simply dropping 0.7 from the tests. I'll do that. |
0.7 passes now |
788ef2b
to
f1d00a6
Compare
Okay, I think with that last push, all tests pass, though there is an error on nightly with
probably due to changes in base. |
I can swap it for 1.0 since it's LTS. Also I'm not sure how to allow the nightly to fail, maybe the line that says
needs to be changed to nightly instead of latest? |
Appveyor hasn't completed yet. I'm a bit rusty and don't remember the syntax for allowing failures on nightly. But a failure on nightly is not a bad thing, I suppose. It looks like something that ought to be fixed. However, for dropping 0.7 it would be deleting the relevant lines: In appveyor.yml:
In .travis.yml: Again, if you don't, I'll do it. But it is tidier to do everything in a pull request. |
You can look now, and the last one also passed, (again, except nightly)
So what is the decision, leave as is? The advantage would be to get a green CI again.
Should I replace 0.7 by 1.0 (the current LTS)? |
I replaced 0.7 with 1.0 & allowed failure on nightly, it can be changed back when the code is fixed. CI should be green now 🤞 |
We'll just merge and tag when we get green checks. This error on nightly requires somebody put the think hat on. It should be a new issue and fixed elsewhere:
Then a new tag before including HTTP 0.9, I think. I don't know if there are interface changes. |
Yes, it looks like this part of the code was copied from base, so I suppose it's a question of copying whatever change was made. As soon as you merge here I'll rebase the other PR. |
all good! ✅ |
I'm not sure exactly when the change in Julia happened so I just set the version to above 1.0 which should be the same as 0.7.
Closes #156