Create distinction between internal velocity macros and API velocity macros

Development
1

Hi everyone,

we make in our Java code a distinction between our internal code that it’s ok to break and change, and our exposed API that is only unstable when it’s new and that we should not break or only with a vote afterwards.
For our velocity code however, we don’t have such distinction, so de facto any macros that we write in a velocity template bundled in XWiki Standard becomes an API.

This is making the work of improving XWiki Standard a bit difficult as we’re not supposed to break those, and we’re supposed to keep maintaining them. So for helping us on the long run, I propose that we start having a distinction between our velocity macros that are exposed as APIs and our velocity macros that are only supposed to be internal.

For making that distinction, we don’t have much choice in velocity other than having a specific naming scheme, and proper documentation.

The proposal here is to prefix every internal macro name with _ to make clear that it’s a technical one and it’s not supposed to be used externally. Then for those macro it would be ok to break them.

So there’s 2 things for which we need an agreement:

  1. Are you ok with the principles of making such a distinction between internal velocity macros and API velocity macros?
  2. Are you ok to use _ as naming prefix for the internal macros?

If we agree on this, I then propose the following plan to apply this rule:

  1. We document this as a new best practice in dev.xwiki.org
  2. We create a task for renaming the internal velocity macros we have in XS, we create a PR with the changes, and we open a vote before merging the PR to ensure everyone is ok with the move to internal of the selected macros. Once merged the change will be documented in the migration notes of the RN. IMO this should be done only in 16.x
  3. In the future, we work on tooling for also having unstable annotation on our API velocity macros and to detect missing doc on them

wdyt?

2

+1

3

+1

Thanks,
Alex

4

Sounds good to me.

Note that we’ll need to create a jira for Loading... to exclude velocity macros starting with _.

PS: Did you check that _ was supported by Velocity as a prefix for macro names? (just in case)

A task where? It might be difficult and not the best to break backward-compatiblity so we’ll have to see this case by case, and mostly focus in the future.

Thanks

5

+1,

Thanks,
Marius

6

+1 for the principle and for the _ prefix.
-1 to rename existing macros, imo they are already part of the API and we can’t change them (at least generally, specific cases can be discussed separately).

7

Thanks for the answers, I documented the new rule for velocity macro naming in https://dev.xwiki.org/xwiki/bin/view/Community/VelocityCodeStyle/.
I drop the proposal for moving existing macros, we’ll do that on a case by case basis following our needs.