-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feature: Update oracledb.js for additional binary path #1602
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
@lemankk please sign the OCA before submitting PRs. Thanks! |
lib/oracledb.js
Outdated
@@ -85,6 +85,9 @@ function _initCLib() { | |||
'./node_modules/oracledb/' + nodbUtil.RELEASE_DIR + '/' + nodbUtil.BINARY_FILE, | |||
'./node_modules/oracledb/' + nodbUtil.RELEASE_DIR + '/' + 'oracledb.node' | |||
]; | |||
if (arg1 !== undefined && arg1.binaryPath !== undefined) { |
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.
Here you can simply check if (options.binaryPath !== undefined)
if you pass options
through to this function.
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.
@anthony-tuininga Thanks for your advice. Changes has been updated for using options directly.
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.
There's no point making changes unless you sign the OCA ! (Sorry!)
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.
Sorry I just noticed that. Already submitted and under review. Do I need to close this PR until review completed?
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.
It fine to leave this open - we have the labels to indicate the overall OCA status
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 like it was approved so now we just need the technical part reviewed.
As a procedural note, we would copy this PR to our private repo, and then sync the changes in that private repo back here. I.e this PR would be closed without any apparent merge but the changes will eventually land.
Thank you for signing the OCA. |
We are working on adding this feature internally with more comprehensive testing and will update you soon. |
@lemankk This PR is now available as a patch in this GitHub repo. Please take the latest version of the file It will added officially in the node-oracledb 6.2 release. Thank you for your contribution! I will close this PR once 6.2 is out. |
Closing this PR as this enhancement has been added in node-oracledb 6.2. |
Hello, this PR is used for create additional parameter
binaryPath
when usinginitOracleClient()
. It's open the way to load oracledb.node from different path of inside the default node_modules folder.Usage: