fix #18 - #19
Conversation
Sorry, something went wrong.
Sorry, something went wrong.
|
@eriwen I added a commit with a 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)
|
Sorry, something went wrong.
|
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)
|
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
| }, done).start(); | ||
| }); | ||
|
|
||
| gulp.task('copy', function() { |
There was a problem hiding this comment.
moved up, the task must be defined before use
Sorry, something went wrong.
| }); | ||
|
|
||
| gulp.task('dist', ['copy'], function() { | ||
| gulp.task('dist', gulp.series(['copy'], function() { |
There was a problem hiding this comment.
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
|
@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, something went wrong.
|
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) |
Sorry, something went wrong.
|
So what do you choose?
|
Sorry, something went wrong.
|
npm scripts would be awesome |
Sorry, something went wrong.
|
@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 |
Sorry, something went wrong.
|
Thanks for your contribution, @caub. I now have some time to give stacktrace.js some love and have merged your PR. |
Sorry, something went wrong.
Hey @eriwen, could yo have a look?