candrews / log4jdbc-spring-boot-starter

Starter for using Log4jdbc with Spring Boot
Apache License 2.0
77 stars 18 forks source link

Consider adding metadata for log4jdbc.* keys #2

Closed snicoll closed 7 years ago

snicoll commented 7 years ago

Rather than having to refer to the README to know the available keys, it would be nice if we could use that in the IDE directly.

It seems a bit artificial to create a @ConfigurationProperties POJO to you aren't going to use ultimately so you could write the metadata manually. Adding a src/main/resources/META-INF/spring-configuration-metadata.json will do.

Example for log4jdbc.auto.load.popular.drivers:

{"properties": [
  {
    "name": "log4jdbc.auto.load.popular.drivers",
    "type": "java.lang.Boolean",
    "description": " Automatically load popular drivers. If this is false, you must set the log4jdbc.drivers property in order to load the driver(s) you want.",
    "defaultValue": true
    }
  ]
}
candrews commented 7 years ago

@snicoll I assume I can't use this mechanism to document the log levels (logging.level.jdbc.sqlonly=FATAL in application.properties, for example).

Can you take a look at https://github.com/candrews/log4jdbc-spring-boot-starter/tree/master/src/main/resources/META-INF/spring-configuration-metadata.json and let me know what you think?

snicoll commented 7 years ago

I assume I can't use this mechanism to document the log levels

That's not even a key. logging.level is and is already documented by us. So I am not sure what you're trying to achieve.

I'll have a look to the link. Thanks!

snicoll commented 7 years ago

@candrews I've reviewed it. IMO, the description in the README is still quite useful (I know it's a pain to have to maintain that in two places). Some description are a bit lengthy but it's your call to make them that way. The file looks good to me.

I assume you've tried that in the IDE to validate they behave as expected?

candrews commented 7 years ago

@snicoll I put the README portion back. I don't like the duplication, but it is easier for users to reference, and that's really the most important thing.

I used STS 3.8.3 to validate. It worked fine, but some types aren't supported very well by STS's autocompletion (ex, the "java.util.regex.Pattern" doesn't have a nice editor/validator, "java.util.list<java.lang.Class<? extends java.sql.Driver>>" doesn't autocomplete matching classes). Hopefully the STS team will keep improving the editor and over time this metadata will become increasingly useful.

Thank you very much for your review, I greatly appreciate it! Does this mean that https://github.com/spring-projects/spring-boot/pull/8758 can be merged now?