-
Notifications
You must be signed in to change notification settings - Fork 203
Quickjs ng 0.11.0 #989
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
Quickjs ng 0.11.0 #989
Conversation
8bfd551 to
a8714a1
Compare
c006222 to
b7ac81b
Compare
BalkanMadman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more request, please. Could you please call pkg-config via a variable ${PKG_CONFIG} that holds the path to pkg-config and defaults to pkg-config.
The use for the variable is cross-compilation where the required pkg-config might not be in path and needs to be specified explicitly.
|
Otherwise, LGTM. |
|
Basically, like this: https://github.com/BalkanMadman/njs/blob/b31b34ae941d19bc13524dfec2519283aa09baa6/auto/quickjs#L10-L12: # Alternative pkg-config binary can be supplied by setting the PKG_CONFIG
# environment variable.
: "${PKG_CONFIG:=pkg-config}"
|
I prefer to not have ad-hoc variables for various tools. You have to add variables anyway, so modifying With other feedback, I do not mind applying your changes. Will do it later. |
Reasonable, thank you! |
c006222 to
2d250c4
Compare
2d250c4 to
9f92555
Compare
BalkanMadman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for polishing the idea! LGTM
VadimZhestikov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
No description provided.