RelTraitPropagationVisitor and useless code policy
Recently, I was looking at the internals of the VolcanoPlanner and I
realized that at various points we make calls to the
RelTraitPropagationVisitor. Currently, the visitor is used only to verify
some assertions. So either it does nothing or it throws AssertionError.
>From my point of view, this seems to be code with no practical use. Given
that the last modification in this part was done in 2014, and no relevant
bugs (AssertionErrors) were reported since then It seems to me that it
could be removed making the life of people reading the code a bit easier.
Apart from the specific example of RelTraitPropagationVisitor, I am more
interested to learn if there is some kind of policy/convention on how to
handle such cases. For instance:
- never delete code
- discuss before PR and delete if necessary
- delete and discuss in the PR
- always delete