Skip to content
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

Update Node version to 8 and 10 #441

Merged
merged 4 commits into from
Apr 3, 2020
Merged

Update Node version to 8 and 10 #441

merged 4 commits into from
Apr 3, 2020

Conversation

keshav234156
Copy link
Member

@keshav234156 keshav234156 commented Mar 29, 2020

Fixes #439 #205

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt jasmine
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • PR body includes fixes #0000-style reference to original issue #
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.

Thanks!

@keshav234156
Copy link
Member Author

@cesswairimu

@cesswairimu
Copy link
Collaborator

@jywarren @emilyashley could you please merge this one...Got no merging rights on this repo

@keshav234156
Copy link
Member Author

@harshkhandeparkar @VladimirMikulic strangely tests are working fine on Node 8 but are failing on Node 10. I think we also have faced a similar issue before #205. Any ideas about this

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Mar 30, 2020 via email

@VladimirMikulic
Copy link
Contributor

VladimirMikulic commented Mar 30, 2020

@keshav234156 since Node v10, fs.writeFile requires a callback function to be passed in. One of the dependencies is not passing the callback and it throws an error.

@keshav234156
Copy link
Member Author

Fatal error: Callback must be a function
The command "grunt jasmine" exited with 3.

@VladimirMikulic
Copy link
Contributor

You would need to dig into node_modules in order to fix this. It's a problem with Jasmine.
Read more here.

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Mar 30, 2020 via email

@keshav234156
Copy link
Member Author

Fatal error: Callback must be a function

is also present in Node 8 and error was also thrown in earlier versions of Node but it didn't failed the travis.

In Node 10 the extra error is

The command "grunt jasmine" exited with 3.

@VladimirMikulic
Copy link
Contributor

I guess our setup is outdated.

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Mar 30, 2020 via email

@keshav234156
Copy link
Member Author

@VladimirMikulic @harshkhandeparkar Fixed by updataing grunt-jasmine-contrib

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Mar 30, 2020 via email

@keshav234156 keshav234156 changed the title Change grunt Update Node version to 8 and 10 Mar 30, 2020
Copy link
Contributor

@VladimirMikulic VladimirMikulic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are ready 👍

@keshav234156 keshav234156 requested a review from jywarren March 31, 2020 16:27
@jywarren jywarren merged commit 5c392b4 into publiclab:master Apr 3, 2020
@jywarren
Copy link
Member

jywarren commented Apr 3, 2020

Looks great folks, thanks a ton!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Travis Failing for all of the latest PRs
8 participants