-
Notifications
You must be signed in to change notification settings - Fork 88
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
Leverage forward declarations to improve compile times #990
Leverage forward declarations to improve compile times #990
Conversation
Are the |
It is not because you can declare things as many time as you want but you can only have one define it once.
Because forward declaration can be problematic especially when using in template classes, so it is encouraged that developers provide this file to let users know what types are okay to use as forward declarations. An example of this is the |
53e7d8f
to
6f3d953
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #990 +/- ##
==========================================
- Coverage 90.91% 90.17% -0.74%
==========================================
Files 280 280
Lines 15876 15701 -175
==========================================
- Hits 14434 14159 -275
- Misses 1442 1542 +100 |
372b9db
to
b6bad0d
Compare
@johnwason Would you have time to look into what the issue is with the windows test failures? |
Why not just use precompiled headers instead of reworking everything? |
At this point I am almost finished updating everything. |
@Levi-Armstrong my fixes didn't work... go ahead and force push to revert my last two commits. |
I am not seeing the serialization unit test failures locally. I am seeing issues with the static loader plugin tests. |
@johnwason This may be related. |
64179cb
to
f9665d4
Compare
I am not seeing the serialization unit tests locally. I am investigating but so far don't have an explanation. Maybe try clearing the github action cache completely? |
4c4d468
to
67e762d
Compare
Can I manually clear the cache? |
You can see all the caches here: https://github.com/tesseract-robotics/tesseract/actions/caches Not sure how to "clear all" though |
I updated to the newest vcpkg boost version and now I am seeing the test failures. I will keep trying to trace the error. |
Looking at the CI, the Conda Windows CI is working with Boost 1.82, while the vcpkg Windows build is failing with Boost 1.84. This may be a problem with Boost 1.84. |
Yeah I am seeing the same failures on the scheduled builds on master. This looks like a boost problem. |
I ran a matrix build of different boost versions, and only 1.84 is failing. https://github.com/johnwason/tesseract/actions/runs/8242279639/job/22540993793 I set the vcpkg revision to use boost 1.83. I recommend opening an issue with boost serialization. |
57295f9
to
b98277c
Compare
@johnwason I believe I removed your change when I force pushed. What change do I need to make to pin the version? |
@Levi-Armstrong I pushed the commit |
See boost/serialization issue boostorg/serialization#309 |
91f5d49
to
c26eb15
Compare
c26eb15
to
5c4cd2b
Compare
1bc9dc4
to
fccd356
Compare
ea3d5c3
into
tesseract-robotics:master
This cuts the build time from 4 minutes and 30 seconds to 2 minutes and 45 seconds.