Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a default implementation of `Configuration` as part of the API #1980

Closed
stellingsimon opened this issue Apr 11, 2018 · 1 comment
Closed

Comments

@stellingsimon
Copy link

@stellingsimon stellingsimon commented Apr 11, 2018

Which version and edition of Flyway are you using?

current master

After removing the fluent-style configuration of #1928 again, it seems there are only two ways to create a Configuration:

  1. use the internal ClassicConfiguration
  2. implement Configuration in a custom class, implementing every single getter method.

ClassicConfiguration is a perfect fit but marked as an internal class, so it's usage is discouraged. I'm not sure why.

The second one is problematic because Flyway upgrades will now necessarily lead to compilation errors when a method is added to the interface, see section 5 of this blog post by Lukas Eder. Most users will only care for a small subset of all configuration options and will happily stick to the defaults for all others. It will also free them from manually copying default values for options they don't care about.

I propose to:

  • rename ClassicConfiguration to BaseConfiguration
  • moving it to an API package,
  • adding something like the following to its JavaDoc:
 * A base class for custom {@link Configuration} implementations in client code.
 * <p>
 * Client code may provide proper {@link Configuration} implementations extending
 * this useful base class. All necessary parts of the {@link Configuration}
 * interface are already implemented. No methods need further implementation.
 * <p>

(The above paragraph was shamelessly plucked from JOOQ's CustomRecord)

I noticed that the current JavaDoc for ClassicConfiguration still references the non-existing FluentConfiguration and the deprecated new Flyway(FlywayConfiguration:

/**
 * Classic JavaBean-style configuration for Flyway. This is primarily meant for compatibility with scenarios where the
 * new FluentConfiguration isn't an easy fit, such as Spring XML bean configuration.
 * <p>
 * This configuration can then be passed to Flyway using the <code>new Flyway(FlywayConfiguration)</code> constructor.
 * </p>
 */
public class ClassicConfiguration implements Configuration {

I'd be happy to provide a PR with the above changes, but I'm not sure this will actually save you any work since the Pro-Edition sections are cut out.

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented May 17, 2018

Both ClassicConfiguration and FluentConfiguration have now made it into the public API

axelfontaine added a commit to flyway/flywaydb.org that referenced this issue May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.