A best practice needs to come from practice, by identifying patterns and extracting them as rules or recommendations.

And this is what is usually done by committers when they vote...

Anyway, the only thing that can be done, and that's valid for any topic, is to make a new proposal/vote. I don't want to discuss the pros and cons of any naming convention here in this jira, it's not the place If/when you do the proposal, it would be good to also find the previous vote thread to see what was said at that time.

EDIT: I've found it: https://markmail.org/message/gkmiix5q5tt7wm7c It points to the VOTE in 2009 at https://markmail.org/thread/xzz2gqmexkgargbz BTW this shows that it's important for committers to participate to votes (we only had 3 committer votes back then in 2009 which is the minimum required).

It clearly shows that we're doing a very bad review job and we as devs are somehow not paying enough attention to our best practices

It can also mean that the rule is not good enough (doesn't take into account all cases) or that it didn't emerge from practice. A best practice needs to come from practice, by identifying patterns and extracting them as rules or recommendations. Otherwise it's artificial. In this case the practice shows that the rule is a bit artificial or rigid. For instance, it feels natural to prefix the configuration property with the name of the module and to use camel case for various parts in the name, but less natural to use camel case for everything below the module name unless you have a real submodule. This is where the rule is a bit rigid.

It becomes natural only after using it regularly

I don't agree. It should emerge from what most of us were already doing (from practice).

PS: I'm reopening the issue so that we don't release 12.7.1 or 12.8RC1 with it by error.

You should check the changes before.

For me, a naming convention or code style is just that. It becomes natural only after using it regularly. The main issue I see is that there's a history and that when introducing a new config name devs might look at existing ones and copy their style rather than follow the defined voted rule.

In any case, right now, this is our voted best practice and we/you should follow it. If you don't like the rule and would like to suggest something else you're more than welcome to do that but it needs to be a proposal on the dev forum and FTM your commit needs to be changed to follow the defined rule (that's why we have these rules). An interesting question to you is: why didn't you follow it? You didn't know about it? You forgot about it? Something else?

I think we should really open again the subject on the forum: personally I didn't recall about those naming rules, and quite frankly I don't really like it. I understand that the naming scheme using the module name is handy for us as developers, but here in xwiki.properties, those names have to be used by XWiki Administrators and they don't really care about the module where those configurations are used, they are more interested about the domains/features on why those properties applies. And yes, as you said a practice that I have personally used is to follow the already existing scheme in this file, because IMO it avoids to lose the users of those properties.
To give you a simple example: I added the property notifications.async.poolSize this property is used in xwiki-platform-notifications-api, problem is that in the same file we have the defined property notifications.rest.poolSize. IMO with such scheme it would have been error prone for users to define notifications.asyncPoolSize.

only 1/3 (6 out of 17) of the configuration properties added to xwiki.properties since the beginning of the 12.x cycle follow the rule:

mflorea It clearly shows that we're doing a very bad review job and we as devs are somehow not paying enough attention to our best practices (either for lack of knowledge or by forgetting them). It could mean that it's time to move to a PR system with at least 1 reviewer before a PR can be merged, even for committers. That's what a lot of other projects do. So far we avoided doing this. Might not be a bad idea even though it probably slows down the speed. FTR this is what Manuel is subject to ATM.

One thing that I regularly see committers do without informing the others and IMO we don't pay enough attention to it, is introducing new APIs/public things (whether it's a java method, a config property, a new URL query string parameter, etc). These are important since it's hard to change them afterwards so it's important that all committers agree about them. A lot of the time committers just add them without making a proposal/pinging the others about them. So I think one way could be either to always do a PR and ask for review when introducing new public things or simply making a proposal on the forum for them.

This proves that the rule is not good. I see 3 "mistakes" that we commonly do:

For me, a naming convention or code style is just that. It becomes natural only after using it regularly. The main issue I see is that there's a history and that when introducing a new config name devs might look at existing ones and copy their style rather than follow the defined voted rule.

In any case, right now, this is our voted best practice and we/you should follow it. If you don't like the rule and would like to suggest something else you're more than welcome to do that but it needs to be a proposal on the dev forum and FTM your commit needs to be changed to follow the defined rule (that's why we have these rules). An interesting question to you is: why didn't you follow it? You didn't know about it? You forgot about it? Something else?

PS: I'm reopening the issue so that we don't release 12.7.1 or 12.8RC1 with it by error.

Thanks

vmassol only 1/3 (6 out of 17) of the configuration properties added to xwiki.properties since the beginning of the 12.x cycle follow the rule:

• refactoring.rename.useAtomicRename (there's no xwiki-platform-refactoring-rename submodule)
• user.store.hint (there's no xwiki-platform-user-store submodule or doesn't use camel case)
• user.preferences.superadmin.<preference name>
• user.preferences.guest.<preference name>
  • there's no xwiki-platform-user-preferences submodule
  • doesn't use camel case
• logging.deprecated.enabled (doesn't use camel case)
• eventstream.store.enabled (there's no xwiki-platform-eventstream-store submodule, or doesn't use camel case)
• notifications.async.poolSize (there's no xwiki-platform-notifications-async submodule)
• edit.document.inPlaceEditing.enabled (there's no xwiki-platform-edit-document submodule)
• solr.remote.baseURL
• solr.remote.corePrefix
  • there's no xwiki-platform-solr module, there is xwiki-platform-search-solr
  • there's no xwiki-platform-search-solr-remote submodule
• openoffice.serverPorts (there's no xwiki-platform-openoffice module)

Note: Unfortunately you'll find some properties not following this rule in xwiki.properties. This is because we've been bad in following this rule and doing code review to ensure it was followed. It was also voted on the mailing list but not documented here in the past.

It's not some, it's the majority of the configuration properties that doesn't follow the rule. This proves that the rule is not good. I see 3 "mistakes" that we commonly do:

• use dot to group configuration properties by domain even when there's no submodule (refactoring.rename.*, user.preferences.*, notifications.async.*, edit.document.*, solr.remote.*)
• use dot instead of camel case to group configuration properties related to a particular topic (.enabled, .hint)
• use a submodule as a top level module (solr.* instead of search.solr.*)

mflorea I'm not sure about the config name. Our rule is defined at https://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HConfigurationPropertyNaming

<module>.<propertyName>

So I would use skinx.jsStringModeEnabled (or something like this) instead.

Added skinx.js.strictMode.enabled configuration to xwiki.properties. For XWiki 12.7 and 12.7RC1 you can disable the strict mode by disabling the JSX minification from xwiki.properties using debug.minify = false. See https://dev.xwiki.org/xwiki/bin/view/Community/Debugging#HDebuggingJavaScript .