Conversation
jacobperron
left a comment
There was a problem hiding this comment.
LGTM
The testing failing in the PR job is an existing flake.
|
Can you clarify what the diff between this PR and the previous branch |
Two main reasons:
|
|
The compilation warnings on macOS are expected; they show up on nightlies as well. The compilation warnings on Windows are also expected; they show up on nightlies as well. What is not expected is the test failures on Windows. The tests that failed looked like they immediately segfaulted on startup, and we do not see that on the nightlies. That needs more investigation before we merge this. |
|
I can reproduce the RViz test failures locally on Windows. Looks like a bunch of "Entry Point Not Found" errors, for example if I run: I get a dialog box containing the following error: I don't really know why this change is causing the tests in RViz to fail. We also see the same test failures happening in Windows CI for #270 (which is an orthogonal change). Maybe it's a difference in our CI job and the nightly? |
356aec5 to
f498434
Compare
f498434 to
c73b6ba
Compare
This will allow overlays on it to work properly. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
c73b6ba to
3131930
Compare
|
I've now rebased this onto the latest. I think that the rviz failures are unrelated, as we are seeing them on all rviz PRs that go through CI (but not on nightlies). So those will have to be investigated separately. I'm going to run CI one more time on this change, just to make sure everything else is good. Given that there have been no substantial changes since the approvals, I'm going to take those as still valid, and will merge assuming CI comes back OK. CI: |
This will allow overlays on it to work properly. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
This will allow overlays on it to work properly. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
This will allow overlays on it to work properly.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
Signed-off-by: Dirk Thomas dirk-thomas@users.noreply.github.com
This should fix #268 .