Update unit tests to support both old and new GTest versions #3677
Conversation
…epending on gtest version
kunaltyagi
added
changelog: enhancement
kunaltyagi
added
needs: code review
|
Sorry, me again with header naming suggestions 🤷♂️ As was agreed in the other discussion (#3654), umbrella-named headers like "xxx_macros" are not desirable. In the case at hand, I believe a more suitable name would be "pcl/test/gtest.h". Indeed, the file is nothing else but include of GTest plus a compatibility layer to support older versions. The fact that this compatibility layer is implemented through macros is not important. |
|
@shrijitsingh99 Incorporate @taketwo's comment regarding file renaming and I'll review it after. |
Renamed the file as @taketwo suggested |
There was a problem hiding this comment.
@PointCloudLibrary/maintainers would you mind if I squash everything up despite the multiple authors? I don't believe this fix is worth more than a single commit.
Edit: by the way. What's the current established label for me waiting for other maintainers to chime? It used to be needs: decision, but that one is no longer part of the label pool.
SergioRAgostinho
removed
the
needs: code review
|
From maintainer perspective yes, please squash. Maybe add contributors' names to the commit message.
We abolished it. Is there a need? What's the difference to "needs: review"? We can introduce "needs: maintainers feedback". |
|
In my mind needing a review means full code review, feedback is to just chime in on a specific topic. While navigating through the issue list it helps me distil both. Did the meaning for needs: review also change? |
No, it's still the same.
OK, understood. I just wanted to compress the label set, which implies a bit of meaning overloading... In this case the difference is too large though. Let's have "needs: maintainers feedback/opinions"? |
|
Just a hint: |
Added the label.
Ah yeah, I guess I'll create an issue of that |
…est versions (PointCloudLibrary#3677) Co-authored-by: Aaryaman Vasishta @jammm <jem456.vasishta@gmail.com> Shrijit Singh @shrijitsingh99 <shrijitsingh99@gmail.com>
taketwo
changed the title
An updated PR which builds upon #3419 and fixes the remaining issues
Fixes #3373 , closes #3419
Credits to @jammm, all of the work was already done by him, I just had to fix some minor issues