search

ros2 / message_filters

BSD 3-Clause "New" or "Revised" License
76 stars 66 forks source link

Improve Explicit Constructors #128

Closed CursedRock17 closed 5 months ago

CursedRock17 commented 6 months ago

As part of pull request #125, cpplint suggests we mark all single param constructors as explicit as it is a good practice for writing c++ code. Technically marking constructors as explicit constitutes an API break, so to prevent cpplint from going off they're marked as NOLINT. An example of an API break from these constructors is with Synchronizer, in which the Policy must be created and initialized first, while much of the community just passes queue_size straight to their constructor. We should find a way to make these constructors explicit, while preventing API breaks

mjcarroll commented 6 months ago

Technically marking constructors as explicit constitutes an API break

I believe it would be fine to go ahead and break API on rolling, we just need to account for it in the next release version increment.

clalancette commented 6 months ago

I believe it would be fine to go ahead and break API on rolling, we just need to account for it in the next release version increment.

I don't generally disagree, though I do think we should do a quick check of downstream packages to see what the extent of the breakage is going to be.