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.

Sign up for GitHub

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

Jump to bottom

fix crypto message #180

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix crypto message #180

wants to merge 1 commit into from

Conversation

rzr
Copy link

@rzr rzr commented Nov 11, 2018

Author: knobik knobiks@gmail.com
Change-Id: If48e0ae4d0ace41052f50accf503a287f2dcb4e4
Origin: jkuri@02fd458
Bug: #175
Signed-off-by: Philippe Coval p.coval@samsung.com

@knobik @rzr
fix crypto message
2d2b428 
Author: knobik <knobiks@gmail.com>
Change-Id: If48e0ae4d0ace41052f50accf503a287f2dcb4e4
Origin: jkuri@02fd458
Bug: JoshKaufman#175
Forwarded: JoshKaufman#180
Signed-off-by: Philippe Coval <p.coval@samsung.com>
@rzr rzr force-pushed the sandbox/rzr/build/review/master branch from 4d82572 to 2d2b428 Compare November 11, 2018 20:57
rzr referenced this pull request in jkuri/ursa Nov 11, 2018
@knobik @jkuri
fix crypto message
02fd458
@rzr rzr mentioned this pull request Nov 15, 2018
Open
@coolaj86
Copy link
Contributor

I’m a little confused as to what this does. At a glance it looks like it removes linking to libssl and libcrypto, which seems like it couldn’t possibly help.

What is the message that this is intended to fix?

How does this fix it?

@coolaj86
Copy link
Contributor

coolaj86 commented Nov 15, 2018 •
edited
Loading

Oops! Reverse that. I see that this ADDS linking.

That said, I’d still like to understand better the problem and what this is doing to fix it.

@rzr
Copy link
Author

rzr commented Nov 15, 2018

Please @jkuri explain how this fixed the node10 support ?

@jkuri
Copy link
Contributor

jkuri commented Nov 15, 2018

NodeJS 10 uses newer version of integrated OpenSSL than previous Node versions do. Because I reformatted existing source code its it is hard to find updates related to OpenSSL version upgrade, but if you check closely you can find them :-).

/cc @rzr @coolaj86

@knobik
Copy link

knobik commented Nov 21, 2018 •
edited
Loading

@jkuri it happened only with electron and node 10 on ubuntu 18

@rzr
Copy link
Author

rzr commented Nov 23, 2018

Is there anything preventing to merge this ? or are we lacking maintainers ?

@rzr rzr mentioned this pull request Nov 23, 2018
Open
@knobik
Copy link

knobik commented Nov 23, 2018

I guess the maintainer is too busy

@rzr
Copy link
Author

rzr commented Nov 23, 2018

Yes it looks like @JoshKaufman has other priorities
that's why I am suggesting to move project to co-maintenance model

#172

Is anyone willing to help on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rzr @coolaj86 @jkuri @knobik