fix #18#19
Conversation
caub
force-pushed
the
patch-1
branch
3 times, most recently
from
1c1c21e to
b506512
Compare
|
@eriwen I added a commit with a test, but |
|
It (primordials) wasn't added until Node 10 or something I think. Tests run on 8 right now. Not at desk but can look later.
…On September 15, 2019 7:05:47 PM UTC, Cyril Auburtin ***@***.***> wrote:
@eriwen I add this test, but `npm test` isn't working for me, there's
probably something to install
```
> gulp test
fs.js:27
const { Math, Object } = primordials;
^
ReferenceError: primordials is not defined
at fs.js:27:26
at req_
(/home/caub/dev/stackframe/node_modules/natives/index.js:143:24)
at Object.req [as require]
(/home/caub/dev/stackframe/node_modules/natives/index.js:55:10)
at Object.<anonymous>
(/home/caub/dev/stackframe/node_modules/vinyl-fs/node_modules/graceful-fs/fs.js:1:37)
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#19 (comment)
|
|
I added a fix commit after googling https://stackoverflow.com/a/55926692/3183756 and gulpjs/undertaker#54 (comment) (I'm using node12, with this commit it should support all node versions) ps: btw I'd rather replace all those gulp tasks by simple npm scripts |
| }, done).start(); | ||
| }); | ||
|
|
||
| gulp.task('copy', function() { |
There was a problem hiding this comment.
moved up, the task must be defined before use
| }); | ||
|
|
||
| gulp.task('dist', ['copy'], function() { | ||
| gulp.task('dist', gulp.series(['copy'], function() { |
There was a problem hiding this comment.
|
@caub What's left for this PR to do? Do we need to upgrade to Gulp 4.x and rewrite gulpfile.js? Or switch to NPM scripts? If we do it one way or another, we need to make it consistent across the entire org. |
|
I upgraded gulp to 4.x on this repo to be able to run tests locally, I can left it to 3.x if you prefer |
|
@caub Could you also upgrade all other repos to 4.x? Would be immensely helpful. I know I'm asking a lot, if you don't have the time I could try to squeeze some in this weekend. |
|
Sorry, I don't want to rewrite anything else to gulp4, I could rather remove gulp on this repo (and use npm scripts), for other repos I think it can be incremental as it's not breaking anything, it's just a devDep (so no need to do it all at once) |
|
So what do you choose?
|
|
npm scripts would be awesome |
|
@niftylettuce ok, great idea, I'll make another PR for this later I've removed the gulp4 commit for this PR, to keep it on topic, and merged |
|
Thanks for your contribution, @caub. I now have some time to give stacktrace.js some love and have merged your PR. |
3 participants
Hey @eriwen, could yo have a look?