-
Notifications
You must be signed in to change notification settings - Fork 337
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
njs: compatibility with 0.8.5 #1293
Conversation
LGTM. |
Do we want to apply this now or wait until the next njs version is released? |
I'd suggest waiting for it until the next njs version is released, perhaps there are more changed APIs. |
For context, the current release is 0.8.4 so we're waiting for something 0.8.5 to get tagged. |
0d0b316
to
02e8afa
Compare
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.
Can we wrap the message after 72 chars please.
Also I'd just make the link a Link:
tag... so something like this
contrib: updated njs to 0.8.5
njs changed strings API so now instead of njs_vm_value_string_set() used
njs_vm_value_string_create() as a drop-in replacement.
Link: <https://github.com/nginx/njs/commit/5730d5ffe23a4965c001d873695d22005fcfa588>
njs changed strings API so now instead of njs_vm_value_string_set() used njs_vm_value_string_create() as a drop-in replacement. Link: <nginx/njs@5730d5f>
02e8afa
to
e57e7fc
Compare
Commit message:
|
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.
I would probably put the package changes in their own commit, but I won't block this on that in this case...
njs changed strings API so now instead of
njs_vm_value_string_set()
usednjs_vm_value_string_create()
as a drop-in replacement.For more information see:
http://hg.nginx.org/njs/rev/4e0553f7ea68