FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: add maintaining info for shared library option by mhdawson · Pull Request #42517 · nodejs/node · GitHub

/ node Public

doc: add maintaining info for shared library option - #42517

Closed
mhdawson wants to merge 26 commits into
nodejs:masterfrom
mhdawson:maint-shared
ConversationCommits (26)ChecksFiles changed
Closed

doc: add maintaining info for shared library option#42517
mhdawson wants to merge 26 commits into
nodejs:masterfrom
mhdawson:maint-shared

Conversation

mhdawson commented Mar 29, 2022

Copy link
Copy Markdown
Member

Refs: #41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in #41850

Signed-off-by: Michael Dawson mdawson@devrus.com

doc: add maintaining info for shared libary option
04fc7e6 
Refs: nodejs#41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in nodejs#41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

nodejs-github-bot commented Mar 29, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/tsc

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 29, 2022
mhdawson mentioned this pull request Mar 29, 2022
Closed
mscdex changed the title doc: add maintaining info for shared libary option doc: add maintaining info for shared library option Mar 29, 2022
gireeshpunathil reviewed Mar 30, 2022
View reviewed changes
Comment thread doc/contributing/maintaining-shared-library-support.md Outdated
Comment thread doc/contributing/maintaining-shared-library-support.md Outdated
mcollina approved these changes Mar 30, 2022
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

mhdawson and others added 11 commits March 30, 2022 09:20
Update doc/contributing/maintaining-shared-library-support.md
31a712d 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
00f1f2b 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
e3d1085 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
e066018 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
2dea68b 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
b81291c 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
8f5935e 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
a5b3e46 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
c721355 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/contributing/maintaining-shared-library-support.md
b3c5a0a 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
squash: address comments
4ce8d4a

mhdawson commented Mar 30, 2022

Copy link
Copy Markdown
Member Author

@mscdex thanks for the review, can't believe I spelled library wrong so many times.

mhdawson commented Apr 7, 2022

Copy link
Copy Markdown
Member Author

@bnb comments addressed.

lux01 reviewed Apr 21, 2022
View reviewed changes
Comment thread doc/contributing/maintaining-shared-library-support.md Outdated
Comment thread doc/contributing/maintaining-shared-library-support.md Outdated
Comment thread doc/contributing/maintaining-shared-library-support.md Outdated
Comment on lines +52 to +61
For the node.exe wrapper, it is built so that it can
find the shared library in one of the following:

* the same directory as the node executable
* ../lib with the expectation that the executable is
installed in a `bin` directory at the same level
as a `lib` directory in which the shared library is
installed. This is where the default package that
is build with the shared library option will
place the executable and library.

lux01 Apr 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this is only true on Linux and macOS where we can embed a relative RPATH into the executable. AIX doesn't have this luxury so currently it hard codes an absolute path to libnode.a into the executable.

On Windows libnode.dll and node.exe sit in the same directory as this is the expected location for DLLs. See this MSDN article if you want to know more about the DLL search order on Windows.

mhdawson Apr 21, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@lux01 updated, can you tell me what the hardcoded path for AIX is so that I can add it?

Comment thread doc/contributing/maintaining-shared-library-support.md Outdated

mhdawson commented May 10, 2022

Copy link
Copy Markdown
Member Author

@lux01 I think this now reflects the current implementation in terms of what was landed. Can you take one more look before I land it?

mhdawson commented May 12, 2022

Copy link
Copy Markdown
Member Author

I'm going to go ahead and land and I'll update if there are any follow on comments.

mhdawson added a commit that referenced this pull request May 12, 2022
doc: add maintaining info for shared libary option
8b415e8 
Refs: #41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in #41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #42517
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>

mhdawson commented May 12, 2022

Copy link
Copy Markdown
Member Author

Landed in 8b415e8

mhdawson closed this May 12, 2022
BethGriggs pushed a commit that referenced this pull request May 16, 2022
doc: add maintaining info for shared libary option
eb630d7 
Refs: #41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in #41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #42517
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
RafaelGSS mentioned this pull request May 16, 2022
Merged
juanarbol pushed a commit that referenced this pull request May 31, 2022
doc: add maintaining info for shared libary option
1d973f0 
Refs: #41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in #41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #42517
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
juanarbol mentioned this pull request May 31, 2022
Closed
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
doc: add maintaining info for shared libary option
5779c14 
Refs: #41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in #41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #42517
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
doc: add maintaining info for shared libary option
2fa3b81 
Refs: #41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in #41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #42517
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
doc: add maintaining info for shared libary option
b559dfe 
Refs: #41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in #41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #42517
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
doc: add maintaining info for shared libary option
688729c 
Refs: nodejs/node#41850

I think it would be good to have some info/context
for maintainers on the shared library option. Add that
based on disussion in nodejs/node#41850

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: nodejs/node#42517
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnb bnb left review comments
richardlau richardlau left review comments
mcollina mcollina approved these changes
gireeshpunathil gireeshpunathil approved these changes
+2 more reviewers
mscdex mscdex left review comments
lux01 lux01 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL