-
Notifications
You must be signed in to change notification settings - Fork 9
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
#1609 clean up workspace before compiling vt-sample-project with vt as TPL #1610
#1609 clean up workspace before compiling vt-sample-project with vt as TPL #1610
Conversation
4620ef2
to
5ff50bb
Compare
PR tests (clang-10, ubuntu, mpich) Build for b5e4b97
|
5ff50bb
to
b6e5873
Compare
b6e5873
to
51373e5
Compare
ci/build_vt_sample.sh
Outdated
fi | ||
|
||
# Don't build vt-sample on Alpine Linux | ||
is_alpine="$(grep ID < /etc/os-release | grep -c alpine || true)" |
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 remove this part and just not call this script on Alpine?
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 can see that you have already omitted the call in alpine-cpp.dockerfile
, so is there any other reason to keep this check?)
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.
Ok, I can see now that this would require a bit more mess in docker-compose.yml
as well. Up to you if you want to go this path.
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.
Absolutely! Done.
I need to take a closer look at this :'- (
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 good to me, I like extracting this to a separate script even just for clarity.
Codecov Report
@@ Coverage Diff @@
## develop #1610 +/- ##
===========================================
- Coverage 83.06% 83.00% -0.07%
===========================================
Files 785 785
Lines 29749 29749
===========================================
- Hits 24712 24692 -20
- Misses 5037 5057 +20
|
Fixes #1609