-
Notifications
You must be signed in to change notification settings - Fork 128
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
Travis Failing for all of the latest PRs #439
Comments
@gauravano @nstjean @crisner @emilyashley any ideas? |
@cesswairimu the dependency uses object restructuring ( |
Upgrading the Node version would be appreciated. |
Thanks @VladimirMikulic |
@keshav234156 take a look |
JS is welcome anytime :) |
Hi guys. Congrats on fixing this. But I would this fix as a workaround rather than a fix. I think not upgrading grunt is not good practice. You should always use the latest version of a package. Also, nodejs version <8 support is stopped. I think we should support only version 8 and 10 (and 12 in the future). We have done that in ImageSequencer. Node 8 and above support ES6 features that are supported even in the browsers and have become standard by now. Using old versions of nodejs and old features of javascript is not advisable. If we desperately need support for older nodejs versions, we will have to write the code with new features, in node 8/10 and use Babel to downgrade the features for use in older browsers/older nodejs versions. Not keeping dependencies/nodejs up to date is one rabbit hole you wouldn't want to go in. Thank you! |
@harshkhandeparkar couldn't agree more! |
@harshkhandeparkar I also agree with you .let's change it to 8 and 10 as of now |
Great everyone...should we open another issue for the fix ☝️ or keep this open? |
@cesswairimu made the changes in the same PR |
Great, thanks, I have pinged Jeff/Emily to merge |
I see most of the recent PRs are all failing. Any help is highly appreciated.
Error
The text was updated successfully, but these errors were encountered: