-
Notifications
You must be signed in to change notification settings - Fork 138
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
[ADDED] Build with GitHub actions #748
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #748 +/- ##
=======================================
Coverage ? 68.76%
=======================================
Files ? 38
Lines ? 15100
Branches ? 3119
=======================================
Hits ? 10384
Misses ? 1680
Partials ? 3036 ☔ View full report in Codecov by Sentry. |
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
@kozlovic Can you please also review? I'll run it in parallel with Travis for a while. The "Release" build can be tested on your own branch if you remove the |
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.
Unless it is required, I would comment the verbose flag from CMakeLists.txt and maybe change the size_t
to int
(unless you have some doc that shows that it ought to be a size_t
.
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
@levb By the way, I forgot to comment at the time, but some of the matrix tests are run 3 times, is it intentional or was it part of some debugging/test at the time you were writing the PR? There is nothing wrong running each test 3 times in a row, it may catch some issues that are timing based, but just wanted to make sure it was intentional. |
Intentional, so see the flappers more often. |
main
main
main
,release_*
check against the latest server release,v2.9.11
; older C compilers, TLS OFF, Streaming OFFCoverage checks should work for new PRs once this is merged to
main
and ran there.