yshrsmz/BuildKonfig

Generating Secondary and more BuildKonfig object

mustafaozhan opened this issue ยท 9 comments

Currently, everything that is generated is added into a single object. That sounds reasonable but we may want to group certain things and seperate from eachother, maybe we can introduce a name into config and use this name to name generated objects

like this

configure<BuildKonfigExtension> {
    packageName = "${ProjectSettings.PROJECT_ID}.common"
    name = "VersionBuildKonfig"

    defaultConfigs {
            buildConfigField(STRING, "VERSION", project.getVersion(), const = true)
    }
}

configure<BuildKonfigExtension> {
    packageName = "${ProjectSettings.PROJECT_ID}.common"
    name = "ApiBuildKonfig"

    defaultConfigs {
            buildConfigField(STRING, "BASE_URL", "www.asd.com", const = true)
    }
}

Additionally, this request will also make sense after we implement: #74 Since we may want to make some Konfigs internal while some public

Can I take on this task?

@l2hyunwoo
Sure, but we first need to design & agree on the new API.
Do you have anything in mind?

I think SQLDelight is a good source of ideas.
https://cashapp.github.io/sqldelight/2.0.0/android_sqlite/

@yshrsmz Oh thanks to provide good reference :)

I skimmed the gradle plugin reference, and it is easy to use I think.

It'll be nice that configuration api should be based on above one. And it'll be better with integrating(or renaming) some properties in this task.

configure<BuildKonfigExtension> {
    packageName = "${ProjectSettings.PROJECT_ID}.common"
    name = "ApiBuildKonfig"
    flavor = "dev(stage, release etc)"
    visibility = INTERNAL/PUBLIC
    

    defaultConfigs {
            buildConfigField(STRING, "BASE_URL", "www.asd.com", const = true)
    }
}

Assuming from your snippet you want to define a buildkonfig extension block for each BuildKonfig object, right?

configure<BuildKonfigExtension> {
  name = "ApiBuildKonfig"
  // ...
}

configure<BuildKonfigExtension> {
  name = "SecretBuildKonfig"
  // ...
}

I'm unsure if we can ensure that BuildKonfig object names are distinct with this setup.

Assuming from your snippet you want to define a buildkonfig extension block for each BuildKonfig object, right?

right!

I'm unsure if we can ensure that BuildKonfig object names are distinct with this setup.

I understood what you worried about. When I was writing above design, I lost that design can't distinct objects brcause of configuration block is separated.

It'll be resolved with below structure.

defaultConfigs {
    register {
        packageName = "${ProjectSettings.PROJECT_ID}.common"
        name = "ApiBuildKonfig"
        flavor = "dev(stage, release etc)"
        visibility = INTERNAL/PUBLIC

        defaultConfigs {
            buildConfigField(STRING, "BASE_URL", "www.asd.com")
        }
    }
    register {
        packageName = "${ProjectSettings.PROJECT_ID}.core"
        name = "CommonKonfig"
        flavor = "dev(stage, release etc)"
        visibility = INTERNAL/PUBLIC

        defaultConfigs {
            buildConfigField(STRING, "BASE_URL", "www.asdd.com")
        }
    }
}

With above structure, configuration block will transform to NamedDomainObjectContainer , so it can distinct object with name.

Reference - SQLDelightPlugin

You may have misunderstood the current API a bit(or just a typo?).

Here's a minimal configuration block for a simple BuildKonfig(current API)

buildkonfig {
    packageName = "com.example.app"

    // default config is required
    defaultConfigs {
        buildConfigField(STRING, "name", "value")
    }
}

If you want to add some flavored config, you can add those by defining targetConfigs.

buildkonfig {
    packageName = "com.example.app"

    // default config is required
    defaultConfigs {
        buildConfigField(STRING, "name", "value")
    }

    targetConfigs {
        // names in create should be the same as target names you specified
        create("android") {
            buildConfigField(STRING, "name2", "value2")
            buildConfigField(STRING, "nullableField", "NonNull-value", nullable = true)
        }

        create("ios") {
            buildConfigField(STRING, "name", "valueForNative")
        }
    }
}

So, you can't nest defaultConfigs, and can't define flavor next to packageName or objectName like you wrote.

Here's my proposal, inspired by yours. What do you think?

buildkonfig {
  // keep the current API for backward compatibility, if possible
  packageName = "com.example.app"
  defaultConfigs {
    // ...
  }

  // below is the new API

  // register method should receive the *required* name parameter so that we can make sure that everyone names their configs.
  register(name = "AppConfig") {
    packageName = "com.example.app"
    // As the name is a required parameter, we can simply provide an option for visibility
    // see the previous discussion: https://github.com/yshrsmz/BuildKonfig/issues/31
    visibility = INTERNAL/PUBLIC

    defaultConfigs {
      buildConfigField(STRING, "name", "value")
    }

    // I still think it's good idea to pass flavor as a parameter for defaultConfigs/targetConfigs
    defaultConfigs(flavor = "dev") {
      // flavored defaultConfigs
    }

    targetConfigs {
      // default targetConfigs
    }

    targetConfigs(flavor = "dev") {
      // flavored targetConfigs
    }
  }

  register(name = "ApiConfig") {
    packageName = "com.example.app"
  }
}

@yshrsmz As you said, it would be good to keep the existing APIs alive for compatibility. Also your new proposal is so reasonable to implement. Thanks for the great suggestion!

@yshrsmz If there's no doubt, I'll start this task, is it okay?

@l2hyunwoo sure, let's do this ๐Ÿ’ช