-
Notifications
You must be signed in to change notification settings - Fork 49
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
Add one master CI which builds, tests and deploys #231
Conversation
Preview page for your plugin is ready here: |
Codecov ReportPatch coverage has no change and project coverage change:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #231 +/- ##
==========================================
- Coverage 82.78% 82.65% -0.13%
==========================================
Files 19 19
Lines 1545 1545
Branches 262 262
==========================================
- Hits 1279 1277 -2
- Misses 191 192 +1
- Partials 75 76 +1 see 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
#-I/usr/include/python2.7 -L/usr/lib/python2.7 # -O3 | ||
LLDBFLAGS = | ||
CXXFLAGS = -c -std=c++11 -m64 -fPIC -I"./btrack/include" \ | ||
CXXFLAGS = -c -std=c++17 -m64 -fPIC -I"./btrack/include" \ |
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.
The change to 17
is required since the change in the location of the eigen repo #229
@quantumjot can you test the deployment? Perhaps when #228 is merged, seeing as you are bumping the version? |
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.
great! it's much better having this run in ci, and a good first step toward using cibuildwheel. One minor change about fail-fast
in ci
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.
Great stuff - a big improvement already. Once this is merged, I can take a look with #228 .
This reverts commit 21b8662.
1589d64
to
631d9f8
Compare
This is a solution to solving #192, however we have not used
cibuildwheels
. Some issues on that to follow (#235), and I'll explain why here.Using
cibuildwheels
to do everything explained in #192 proved quite complex and would involve a lot of changes. This PR has the minimal changes which respect the current workflow but remove the libraries fromgit
. I will raise an issue separately about Windows shortly (#234). We (@p-j-smith and I) hadn't realised that the Windows binary was cross-compiled, so a bit of pain trying to fix that.The main thing done here is that now where we previously had a testing workflow and a deployment workflow, we now build the C++, test the python and deploy in a single workflow. This should ensure we don't have a failing deployment.