Update dependencies #1519
Update dependencies #1519
Conversation
Croydon
force-pushed
the
node10
branch
2 times, most recently
from
0df376a
to
d1fd091
Compare
Croydon
force-pushed
the
node10
branch
13 times, most recently
from
cf000d5
to
00db706
Compare
Croydon
force-pushed
the
node10
branch
2 times, most recently
from
5c18164
to
e31a4f4
Compare
|
Node.js 6 on AppVeyor is exceeding the time limit of 1h and I'm not sure why. Has someone any idea? |
|
Node 6 on Windows: It seems to always freeze at the test step. At or after (not sure, likely after as the output indicates that the test is successful) the commit Can someone have a look who is more familiar with nodegit? |
.travis.yml
Outdated
| - master | ||
| - /^v\d+\.\d+\.\d+$/ | ||
| sudo: true | ||
| dist: xenial |
There was a problem hiding this comment.
We shouldn't update to xenial until April of 2019. Ubuntu 14 is our minimum support until LTS ends.
There was a problem hiding this comment.
Really? Xenial seems to be pretty standard now. Is it possible that trusty would work and xenial would fail? Which would be more likely.
There was a problem hiding this comment.
It's more likely that xenial would work and trusty would fail. My main concern is that libcurl dependency for proxy support being dynamically linked.
Croydon
force-pushed
the
node10
branch
2 times, most recently
from
e56b722
to
7f4fd05
Compare
|
This looks promising, I'll focus attention on this branch. How did you get the test error: |
Croydon
force-pushed
the
node10
branch
4 times, most recently
from
efc8fe6
to
9c7ef07
Compare
|
Okay, after who knows how many rebases, commits and resets this should be merge ready. |
|
The tests failed, so I just restarted them to see if it's some of that appveyor inconsistencies I've seen with compiling. |
|
Oh no. That seems like the same bug as CI had before at some points. Node 6 on Windows freezes in some test and never resumes. Seems like some dependency update isn't compatible with Node 6 or brings another behaviour as on the newer Node versions. |
|
So, the freezes are coming from the promisify-node commits (first two)... |
|
I have ripped out the promisify-node updates, now CI should give green light |
appveyor.yml
Outdated
|
|
||
| branches: | ||
| only: | ||
| - master |
There was a problem hiding this comment.
Correct me if I'm wrong, but doesn't the removal of this line cause every branch on the nodegit repo to be built / tested on appveyor?
There was a problem hiding this comment.
Yes, and in every fork with has activated AppVeyor.
Having the CI running only in pull requests in annoying.
There was a problem hiding this comment.
I don't think I am willing to approve this change. For instance, some maintainers, not just myself, will push their branches to the nodegit repo, and then make PRs against master. In those instances, we will be double building branches for no reason. It will also clutter up our appveyor / travis test results.
Is it too much to ask to create a pull request when you are looking to test against CI? I'm sorry that you feel that's too annoying!
There was a problem hiding this comment.
At best we could split this and the travis change out, and come back to it in a separate PR where we can have more discussion about it.
as it is EOL
Dear future contributors, please update this in April 2019 when Trusty becomes EOL. Keep extended testing and documentation upload on Xenial as it is easier there to install required dependencies and other nice perks. Also don't allow Node 10 to fail anymore.
|
Removed commits which lets the Travis and AppVeyor run on all branches, even though I would strongly recommend it for the future. |
|
Thanks @Croydon! |
Closes #1499
Closes #1237
Closes #1547
Closes #1534
Closes #1543
Closes #1490
Fixes #1539
I couldn't get #1365 & #1506 to work as these commits always froze Node 6 testing on Windows.