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

add docs for cpp tests requiring built libraries #4401

Merged
merged 2 commits into from
Aug 2, 2022
Merged

add docs for cpp tests requiring built libraries #4401

merged 2 commits into from
Aug 2, 2022

Conversation

williamberman
Copy link
Contributor

Description
Update contributor guidelines to note cpp tests require linking to built libraries (i.e. ONNXIFI_DUMMY_LIBRARY).

Motivation and Context

  • The cpp tests fail when running locally for me.
  • The ci scripts also set LD_LIBRARY_PATH

@williamberman williamberman requested a review from a team as a code owner August 1, 2022 21:49
williamberman
williamberman commented Aug 1, 2022
View reviewed changes
docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
jcwchen
jcwchen reviewed Aug 1, 2022
View reviewed changes
Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

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

Comments about Windows part. The part of Linux and MacOS LGTM. Thanks for the improvement !

Please follow the steps to signoff DCO.

docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
@williamberman williamberman force-pushed the will/ld-library-path-docs branch from 1ddc87d to b7ed691 Compare August 1, 2022 23:40
@williamberman williamberman requested a review from jcwchen August 1, 2022 23:41
jcwchen
jcwchen reviewed Aug 2, 2022
View reviewed changes
docs/CONTRIBUTING.md Outdated Show resolved Hide resolved
@williamberman @jcwchen
add docs for cpp tests requiring built libraries
f3708d9 
add jcwchen change for windows

Signed-off-by: Will Berman <[email protected]>

Update docs/CONTRIBUTING.md

Co-authored-by: Chun-Wei Chen <[email protected]>
@williamberman williamberman force-pushed the will/ld-library-path-docs branch from fd39637 to f3708d9 Compare August 2, 2022 17:27
@williamberman williamberman requested a review from jcwchen August 2, 2022 17:28
jcwchen
jcwchen approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jcwchen jcwchen enabled auto-merge (squash) August 2, 2022 17:35
@jcwchen jcwchen added the topic: documentation Issues related to ONNX documentation label Aug 2, 2022
@jcwchen jcwchen merged commit 2071a27 into onnx:main Aug 2, 2022
broune pushed a commit to broune/onnx that referenced this pull request May 6, 2023
@williamberman @jcwchen
add docs for cpp tests requiring built libraries (onnx#4401)
db758e3 
add jcwchen change for windows

Signed-off-by: Will Berman <[email protected]>

Update docs/CONTRIBUTING.md

Co-authored-by: Chun-Wei Chen <[email protected]>

Co-authored-by: Chun-Wei Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcwchen jcwchen jcwchen approved these changes

Assignees
No one assigned
Labels
topic: documentation Issues related to ONNX documentation
Projects
PR Tracker
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@williamberman @jcwchen