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

fix mac build issue for tracevis #258

Merged
merged 1 commit into from
Apr 29, 2020
Merged

fix mac build issue for tracevis #258

merged 1 commit into from
Apr 29, 2020

Conversation

aman1309
Copy link
Contributor

@aman1309 aman1309 commented Apr 24, 2020

Upgrading phantomjs to solve one of the build issue in testCov, fails on mac only since c++ libs used to build are different on mac. Other issues relating to installation failure:

Root cause is contextify being no longer support as it is merged native to node.
Permanent fix is upgrade node and package dependencies.

for now will have use below patches(one time). I will add specific cmds to developer guide.

Refer:
nodejs/node-gyp#569
nodejs/node-gyp#469

Copy link
Contributor

@junchuanwang junchuanwang left a comment

Choose a reason for hiding this comment

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

After offline chat, this looks good to me. Maybe reword the description to explain a little bit about the relationship between this version bump and commands you are going to add to the developer guide.

@@ -9,7 +9,7 @@
"jshint": "~2.1.11",
"istanbul": "~0.1.44",
"mocha": "1.2.x",
"phantomjs": "~1.9.2-2",
"phantomjs": "^2.1.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

For my own understanding, what does ^ mean here? 2.1.7 or above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

equivalent to 2.*
~2.1.7 is 2.1.*

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Thanks.

@aman1309 aman1309 requested a review from mchen07 April 24, 2020 16:33
@evanw555
Copy link
Contributor

I would advise that you leave a note in the README.md file for :parseq-tracevis showing the exact commands needed to fix this. Not everyone has access to our internal development guide, plus this would make the information easier to access for anyone trying to build parseq.

@aman1309
Copy link
Contributor Author

I would advise that you leave a note in the README.md file for :parseq-tracevis showing the exact commands needed to fix this. Not everyone has access to our internal development guide, plus this would make the information easier to access for anyone trying to build parseq.

I think we should add a dev guide which can include this, will add this task in queue

@aman1309 aman1309 closed this Apr 29, 2020
@aman1309 aman1309 reopened this Apr 29, 2020
@aman1309 aman1309 merged commit dc3ab93 into master Apr 29, 2020
@aman1309 aman1309 deleted the fix branch April 29, 2020 06:19
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.

4 participants