slachiewicz commented on PR #1073:
URL: https://github.com/apache/maven-doxia/pull/1073#issuecomment-5226726861

   Pushed a890a946 and expanded the description after a closer look at what the 
subclass approach actually costs. Three things worth surfacing rather than 
leaving for review to find:
   
   **The constants change runtime type.** Since the static initialisers now run 
in the new class, `impl.SinkEventAttributeSet.BOLD` and friends, and every 
attribute set Doxia produces internally, are instances of the new class only. 
`instanceof` against the old class returns `false` where it used to return 
`true`. japicmp cannot see this, and neither could I until I probed the built 
jars. The alternative is redeclaring the constants on the deprecated class, 
which keeps the old type but costs their identity across the two names. I 
picked identity and pinned it with a test, but it is a judgement call, so 
please say if you would rather have it the other way.
   
   **"No exclusions" was wrong** in my original description; doxia-core already 
carries one for `SinkUtils#filterAttributes`. This PR adds none of its own. 
Corrected.
   
   **The parser hooks make the deprecation unfulfillable in 2.x**, since the 
class cannot be removed while they reference it. Filed #1074 for retyping them 
to `SinkEventAttributes` in the next major and dropping the class after, so 
this is staged rather than half-done.
   
   Also added `@since`/`@deprecated` version markers, on the assumption this 
targets 2.2.0 rather than 2.1.1.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to