Quarkus: Renaming enable properties to enabled

[agoncal]

Description

While debugging I noticed that I misspelled a Quarkus property. I wrote enabled instead of enable. Then, I realized that Quarkus uses both... and it's confusing. It looks like it's just a consistency problem (as enabled is more frequently used).

Here are the properties that should be renamed

quarkus.keycloak.policy-enforcer.enable

quarkus.log.console.enable
quarkus.log.file.enable
quarkus.log.syslog.enable
quarkus.log.handler.console."console-handlers".enable
quarkus.log.handler.file."file-handlers".enable
quarkus.log.handler.syslog."syslog-handlers".enable

quarkus.swagger-ui.enable

[lower-case]

Hi, I'm new to Quarkus and I'd like to work on this enhancement. I checked the code and got a basic idea of what to fix.
I've a doubt, Do we have to provide backward compatibility for existing projects using these properties?
Please advise.

[geoand]

Hi,

Yes, we absolutely do need to maintain backwards compatibility - at least for a while.
Which means that initially we need to have both properties.

[rsvoboda]

@geoand / @gsmet how should be such dual compatibility modes tracked? And for how long?

Is @Deprecated with some comment good enough for configuration properties?

[geoand]

I don't really know how long we should keep both of them around, but I would guess for at least two releases since the new property's introduction.

And yeah, I think a comment thinking to the new property would be enough. It should show up in the generated documentation automatically.

[machi1990]

And yeah, I think a comment thinking to the new property would be enough. It should show up in the generated documentation automatically.

We do not generate doc for deprecated properties. But I think we should at least log a warning for deprecated properties - during build for build time configs and boottime for runtime configs. I am seeing this https://github.com/quarkusio/quarkus/blob/43ab7d436cc3442945d33259e5eeba59d4422d3b/core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigDiagnostic.java#L46-L48 but not sure if it works as I do not see it being used anywhere /cc @dmlloyd

[dmlloyd]

Cases like this are probably best handled by using a redirecting interceptor so we don't have to introduce redundant methods with lots of extra checking.

We could add an annotation like @RenamedFrom({ "oldName1", "oldName2", ... }) and the interceptor could detect the old names, log a warning, and rename the property before the config system gets it.

[lower-case]

Hey everyone, I've raised a draft PR for changes suggested by @dmlloyd. To keep it simple, I've added a new field in the current annotation instead of creating a new annotation. I request you all to kindly review my PR and provide your suggestions.
I also did some extra minor changes in ConfigInstantiator.java to get rid of some warnings by my IDE. Please suggest If I should remove them.

Thanks in Advance!!

[radcortez]

This is easily supported by SmallRye Config 1.8:
https://smallrye.io/docs/smallrye-config/interceptors/interceptors.html#relocate-interceptor