Design more convenient API

[krzema12]

https://kotlinlang.slack.com/archives/C02UUATR7RC/p1643817358166909?thread_ts=1643817358.166909&cid=C02UUATR7RC

[LouisCAD]

I'd take inspiration from kotlinx.coroutines where they keep deprecated symbols for the naming people know from Rx, with ReplaceWith clauses that rewrite correctly with the differently named symbol for Flow.

That way, people that know the YAML schema can jump right through.

BTW, I think that "uses" is only a key in a json object, that is part of the steps json array, so it should probably be the name of a parameter for a step function rather than the name of the function itself.

[krzema12]

Here's a draft of the new API, taking into account some of the existing problems. Just read from top to bottom:

val myWorkflow = workflow(
    name = "Test workflow",
    on = listOf(Push()),
    sourceFile = Paths.get(".github/workflows/some_workflow.main.kts"),
    targetFile = Paths.get(".github/workflows/some_workflow.yaml"),
) {
    job(
        name = "test_job",
        runsOn = RunnerType.UbuntuLatest,
    ) {
        // Current API:
        uses(
            name = "Check out", // Name is mandatory.
            action = CheckoutV2(),
        )

        // Proposed equivalent API:
        checkoutV2() // Simple function calls. No name mandatory, especially for simple actions.

        name("Check out") // But the name still possible to be added like this.
            .checkoutV2()

        // Wanna pass some action arguments? Just use function arguments:
        checkoutV2(
            fetchDepth = FetchDepth.Infinite,
            sshStrict = true,
        )

        // What about conditions? Use 'condition':
        name("Always do something")
            .condition("\${{ always() }}")
            .doSomethingV3(
                someArgument = "someValue",
            )

        // 'run' gets a new look as well:
        env(linkedMapOf(
                "SIMPLE_VAR" to "simple-value-job",
                "MULTILINE_VAR" to """
                hey,
                hi,
                hello! job
                """.trimIndent()))
            .run("""
                 echo 'run your commands here'
                 """.trimIndent())

        // A pattern emerges: all optional step items are set with chained methods. It forces putting them in
        // front and calling the action as the last piece, which effectively sets a convention - your brain will
        // eventually learn to read the optional parts first, and to look for action call at the end.

        // What about outputs? Given we call actions as functions, we shall get the outputs equally easily:
        val myStep = someActionProducingOutputsV2(
            someArgument = "someValue",
        )
        // Then use the placeholders this way:
        run("""
            echo 'this is the value of the output:: \${{ ${myStep.outputs.fetchDepth} }}'
            """.trimIndent())
        // ${myStep.outputs.fetchDepth} would resolve to a string being the GitHub Actions placeholder.

        // What if I want to pass an output of one action as an input of another action, and this input is not
        // of type string (cannot simply use string interpolation)?
        // Then go with '..._unsafe' variant of action wrapper function where all arguments are strings:
        checkoutV2_unsafe(
            fetchDepth = "\${{ myStep.outputs.fetchDepth }}",
            someArgument = "I can pass whatever I want, and Kotlin type-safety won't type-check it for me",
        )
        
        // Another alternative to have any sense of typing is to go with typed providers for each input.
        // E.g. assuming some input in checkoutV2 of type Int, in checkoutV2_unsafe it would be e.g. Value<Int>:
        checkoutV2_unsafe(
            fetchDepth = { "\${{ myStep.outputs.fetchDepth }}" },
        )
        // The benefit would be that if only one input has to be set using string interpolation, the rest would
        // still have some type safety, and the compiler would ensure that we pass Value<Int> we got as output
        // to an input of type Value<Int> as well.
    }
}

@jmfayard @LouisCAD @madhead @msfjarvis @NikkyAI I'm open to your feedback and observations!

[jmfayard]

I designed a Step api here:

https://github.com/jmfayard/refreshVersions/blob/800055ca10d8c6a654365d69d05903730e5fc177/.github/workflows/build.main.kts

fun JobBuilder.step(step: Step) =
    uses(step.name, step.action, LinkedHashMap(step.env), step.condition)

data class Step(
    val name: String,
    val action: Action,
    val env: Map<String, String> = emptyMap(),
    val condition: String? = null
) {
    fun env(env: Map<String, String>) = copy(env = env)
    fun condition(condition: String) = copy(condition = condition)
}


fun Action.named(name: String) = Step(name, this)

used like this:

val workflowRefreshVersions = workflow(
    name = "RefreshVersions PR",
    sourceFile = Paths.get("build.main.kts"),
    targetFile = Paths.get("refreshVersions_pr.yml"),
) {
    job("check-build", RunnerType.UbuntuLatest) {

        step(Steps.checkout("main"))
        step(Steps.setupJava)
   }
}
   
object Steps {

    fun checkout(branch: String? = null): Step =
        CheckoutV2(
            ref = branch,
        ).named("check-out")

    val setupJava: Step =
        SetupJavaV2(
            distribution = Adopt, 
            javaVersion = "11",
        ).named("setup-java")
 }        

Reason:

• The action and its name belong togther (hence: Step) and I can reuse them in muliple workflows
• I don't want to write extension functions of JobBuiler everywhere

[NikkyAI]

i'd prefer env as a optional parameter on run, that is less indentation and more readable

so like this:

run(
    command = 
        """
            echo 'run your commands here'
        """.trimIndent(),
    env = mapOf(
        "SIMPLE_VAR" to "simple-value-job",
        "MULTILINE_VAR" to """
              hey,
              hi,
              hello! job
            """.trimIndent()
    )
)

this makes reusing the env map easier to read in the code as well

as for steps.. i think i prefer the current api to most of the suggestions

the api could be changed to make name optional and if not provided.. then compute it from the provided action

val buildJob = job(name = "build_job", runsOn = UbuntuLatest) {
	uses(
        name = "Check out",
        action = CheckoutV2(
            fetchDepth = CheckoutV2.FetchDepth.Value(0)
        )
    )

	// or simpler:
	uses(
        CheckoutV2(
            fetchDepth = CheckoutV2.FetchDepth.Value(0)
        )
    )
}

could also be named step as in @jmfayard 's suggestion , i also like the reusability of steps in that suggestion

[jmfayard]

Note about the re-usability: I generate multiple YAML files with one script. I have three files "Deploy to development", "Deploy to staging", "Deploy to production" and they only differ by the name, the branch they operate on and the GitHub secret they use.

[NikkyAI]

chaining calls is nice.. but why not have the optional modifications at the end modify the thing and add the whole result to the steps ? that makes it possible to construct them elswhere (via extension maybe) and/or reusing the constructed steps possible..

steps += uses(SomeAction())
		.withEnv(mapOf("hello", "world")).
		.withName("action")
		.withId("action")
		.withArguments(
			mapOf("argument" to "value")
		)

those extension functions might as well just call copy internally

[krzema12]

Thanks for the feedback! Because your opinions are somewhat contradictory, let me hold with this until I have more data, perhaps when there're more users of the library and I analyze what kind of helper functions for API adjustment they create. In the meantime, I'll focus on extending the library with new features that may influence the final API.

[krzema12]

See [#145] feat(library): Make step name optional, it's an improvement within what we discussed about possibly new API.