-
Notifications
You must be signed in to change notification settings - Fork 656
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
prepare ci scripts #32
Conversation
RUN update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-5.0 100 | ||
|
||
# modern curl | ||
#RUN apt-get install -y curl |
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.
remove ? Why do we need some special curl ?
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 is a good question for @weissi, as far as i can tell the integration tests require more modern curl with uds?
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.
@tomerd integration tests will work with any curl that does UDS.
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.
oh, we're basing this on Ubuntu 14.04. IIRC that curl does not do UDS...
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.
so I believe this is neccessary at the momen
docker/Dockerfile
Outdated
RUN apt-get install -y libicu-dev libblocksruntime0 pkg-config | ||
RUN apt-get install -y lsof dnsutils | ||
|
||
RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key|sudo apt-key add - |
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.
nit. whitespace before and after the pipe.
docker/Dockerfile
Outdated
|
||
# script to allow mapping framepointers on linux | ||
RUN mkdir -p $HOME/.scripts | ||
RUN wget -q https://raw.githubusercontent.com/apple/swift/1e078fbdfa768e211e0473cf917511efd73aec86/utils/symbolicate-linux-fatal -O $HOME/.scripts/symbolicate-linux-fatal |
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.
nit: Should we just merge the script into our repo ?
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.
@weissi wdyt? i think its used elsewhere also?
Motivation: set up continuous integration Modifications: * update docker file to include all dependencies required for build, unit tests, integration tests and doc generation * remove dependency steps from doc generation script * add integration tests driver script Result: able to set up docker based continuous integration
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 fine to me, but leaving to @weissi to review.
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.
lgtm, thanks @tomerd
Motivation: SMTP in 2019 uses STARTTLS, therefore NIOSMTP should support that too. I have also been asked multiple times if NIOSMTP supported STARTTLS. Modification: Support STARTTLS Result: Better example.
Motivation:
set up continuous integration
Modifications:
Result:
able to set up docker based continuous integration