Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
build plugin in docker #65
build plugin in docker #65
Changes from 62 commits
040c19a
edc9df4
4190b46
52eb2f8
f1e19b8
db81d47
25d72d6
ccc3496
25fafaa
51c89fc
6b60ceb
1a8fd85
c150a0f
ecf20b7
dfde610
2d38516
1536422
1fe4574
c8ee9f9
0449999
607faf7
e0b018b
afa2021
e317672
18aaf02
92294ef
78f69cb
c535f29
90f6c76
e4e41bb
37d798e
34acb58
9f8a7cb
4e75bc9
4b718b5
8809c89
040918f
ccd3a9c
3880c48
b0b451d
3caa7ad
d972a8f
32488ca
ab2cd9b
5573a6c
07f8c48
7025f37
2d0c4e2
80c3206
54d1b6b
cc15fa9
90e1193
a97af63
fa6a506
ac1ff2b
86e84c5
0a1c36b
f7db14b
ef5d3ea
296a66b
abce1a2
6e3a63f
a8b7d00
f01e71d
32969ad
ea8bb9e
e8ece15
51a10b3
6e851e1
c45e9bd
e490828
7246373
9e0cf2a
2d707d6
d3b64a9
f92ab48
5f83026
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 know you still have documentation to work on, but can we add a comment here for what we expect the argument,
$1
, to be?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 don't see the
LD_LIBRARY_PATH
being set anywhere else, so I think this can be removed.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.
if you run this on your computer one want to be sure that the LD_LIBRARY_PATH is not set to ovoid conflict...
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.
@pshriwise I believe this is the only outdenting comment.
BU this was done on purpose for safety....
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.
Gotcha. I was understanding these as intended for use in docker only, meaning we'd have a fresh environment. Nice to be able to use them in the native kernel though -- have we tested that?
All that aside, would this command be more appropriate in the
build_plugin.sh
file? It would be more clear that theLD_LIBRARY_PATH
is unset by these scripts I think.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.
This repo and branch can be updated with the merge of your PR in DAGMC
svalinn/DAGMC#717
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.
We use this line in multiple places, let's make the number of make jobs a variable.