-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
2096244
to
b9eb997
Compare
Cooool! When can we get this merged? |
The GitLab build for this PR took 86 minutes. Was that expected? https://gitlab.com/ipfs-ci/js-ipfs/pipelines/38534308 |
No!! node 11 takes a long time dunno why yet probably our fault. And remember this is running in gitlab linux shared containers , win/mac in a single mac book pro (win inside virtualbox) and no cache tweaks like travis has (ill add that later). TLDR ignore how long it takes and look at how it works, features, interface. Pipeline duration can only improve with dedicated machines and cache. |
I have a couple questions, but I think since the plan is to test this for a while in parallel, these aren't blockers for this PR.
Overall the individual job reporting at the pipeline level is much nicer than we have now, and individual retries is fantastic. |
It does seem like a vast improvement 😄 |
.gitlab-ci.yml
Outdated
- coverage/ | ||
|
||
test-browser-firefox: | ||
allow_failure: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this meant that the overall build will pass even if this step fails? What's the reason for adding this currently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. dunno its a bug we break firefox in this test
.key.gen
✓ should generate a new rsa key (11404ms)
.gitlab-ci.yml
Outdated
@@ -0,0 +1,89 @@ | |||
image: hugomrdias/node-alpine:test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we publish this under "ipfs" or "protocol labs" or something? What's the difference between this image and regular node-alpine
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we think about that after we test ?
FROM node:10-alpine
RUN apk add --no-cache \
libstdc++ \
binutils-gold \
g++ \
gcc \
gnupg \
libgcc \
linux-headers \
make \
python \
git \
curl
.aegir.js
Outdated
mocha: { | ||
bail: true, | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will some of this eventually move into Aegir? Seems weird that we have so much Aegir config overrides when it is basically built to service PL JS projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are adding features here that aegir doesn't support yet, but it will and this will all be removed.
@hugomrdias are you happy to merge this or is there more you want to add? |
i would like to have at least chrome green, it seems the error is network related i did a restart lets see |
cb1da9d
to
9c0ccbd
Compare
8a17b68
to
617a927
Compare
@hugomrdias could I get an update on the blockers for this now? |
@hugomrdias can't wait for you to put this one on a boat 🛳 \o/ :D |
fix: update lock fix: ignore packages fix: fix deps and add lock fix: fix deps 2 fix: ignore packages 2 fix: add mocha reporter to karma fix: remove unused cmd chore: update lock feat: add gitattributes feat: test attributes feat: test attributes 2 fix: fix stderr match fix: fix more files line endings fix: fix line ending chore: remove lock chore: fix win test run chore: ignore lock chore: change to yarn chore: fix cov chore: fix cov
4972dcf
to
68420ff
Compare
Needs:
ipfs/aegir#324