Using the criticality feature of Revapi

Hi devs,

We currently break the build on binary and semantic incompatibilities. There are 4 types of cases I can see:

  1. Real breakages that we still want to do but the users need to be warned since existing code using these APIs will or may fail at runtime.
  2. Real breakages but happening on @Unstable code.
  3. Semantic changes that are potentially breaking in severity(like adding an annotation) but that, after analyzing we consider to not be a breakage.
  4. Revapi bugs or limitations, that are not breakages in our opinion

I’m proposing to use the criticality feature of revapi to differentiate them when adding the differences items in our pom.xml, as follows:

  • For 1), use the highlight criticality
  • For 2), use the documented criticality
  • For 3) and 4), use the allowed criticality

Then, the {{backwardCompatibilityReport134/}} macro would be modified to display highlight and documented in two different lists, and to skip allowed ones (i.e. they would not be shown to the user at all).

WDYT?

Thanks

Sounds good, +1.

FTR definition of criticality: Architecture :: Revapi

+1 Thanks

LGTM +1.

Just a side note, I was checking how we would put the ignore with different criticalities in our poms, AFAICS it’s defined in revapi.differences (looking in https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/pom.xml#L107), so it would mean one revapi.differences node per criticality in the pom? Or as many revapi.differences as “grouped changes” (e.g. we would group the ignore by justification, and put the criticality on them)? It’s not that clear to me while looking the pom that we can have multiple revapi.differences node.

[EDIT: just noticed that it can be put in the item node actually like shown in https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/pom.xml#L131 so I guess we’d do that]

Yep, exactly.

+1, sounds good.

Thanks,
Marius

First part done: documented at Development Practices (Community.DevelopmentPractices) - XWiki

Next part: update the {{backwardCompatibilityReport134/}} macro.

Could also be APIs moved into another Maven module (I’ve updated the doc at Development Practices (Community.DevelopmentPractices) - XWiki )

BTW the proposal has been agreed so please start using this! (we don’t check automatically that the criticality field is filled ATM)

Just tested and we can have several differences groups, such as:

            <revapi.differences>
              <justification>Code has been moved to another Maven module without any breakage</justification>
              <criticality>allowed</criticality>
              <differences>
                <!--item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.DefaultSecurityReferenceFactory</old>
                </item-->
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.GroupSecurityReference</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.SecurityReference</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.SecurityReferenceFactory</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.UserSecurityReference</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.AccessDeniedException</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.AuthorExecutor</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.AuthorizationException</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.AuthorizationManager</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.AuthorizationManagerConfiguration</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.AuthorizationSettler</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.ContextualAuthorizationManager</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.DefaultAuthorizationManager</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.EntityTypeNotSupportedException</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.Right</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.RightDescription</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.RightMap&lt;V&gt;</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.RightSet</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>enum org.xwiki.security.authorization.RuleState</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.SecurityAccess</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.SecurityAccessEntry</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.SecurityEntry</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.SecurityEntryReader</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.SecurityEntryReaderExtra</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.SecurityRule</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.SecurityRuleEntry</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.UnableToRegisterRightException</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.cache.ConflictingInsertionException</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.cache.ParentEntryEvictedException</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.cache.SecurityCache</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.cache.SecurityCacheLoader</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.cache.SecurityCacheRulesInvalidator</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>interface org.xwiki.security.authorization.cache.SecurityShadowEntry</old>
                </item>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.authorization.event.RightUpdatedEvent</old>
                </item>
              </differences>
            </revapi.differences>
            <revapi.differences>
              <justification>Code has been moved to another Maven module without any breakage</justification>
              <criticality>allowed</criticality>
              <differences>
                <item>
                  <code>java.class.removed</code>
                  <old>class org.xwiki.security.DefaultSecurityReferenceFactory</old>
                </item>
              </differences>
            </revapi.differences>

Note: We need to upgrade our release tool though to support it…