sharpliner / sharpliner

Use C# instead of YAML to define your Azure DevOps pipelines
https://www.nuget.org/packages/Sharpliner/
MIT License
285 stars 21 forks source link

How to use BooleanParameter as value in task? #266

Closed mathiasi closed 1 year ago

mathiasi commented 1 year ago

I've got this example:

Parameters =
{
    BooleanParameter("myBool", "", false),
},
Jobs = new() {
    Job("job") with
    {
        Steps = new()
        {
            DotNet.Test("*Test.csproj") with
            {
                DisplayName = "Dotnet Test",
                TestRunTitle = "Title",
                Arguments = "--no-build --configuration Release --collect \"Code coverage\"",
                ContinueOnError = parameters["myBool"]  // <--- ERROR: Cannot implicitly convert type 'Sharpliner.AzureDevOps.ConditionedExpressions.ParameterReference' to 'bool'

            }
        }
    }
}

Is there a way to pass the parameter on to the input of the task?

mathiasi commented 1 year ago

Only workaround I could think of was

DotNet.Test("*Test.csproj") with
{
    DisplayName = "Dotnet Test",
    TestRunTitle = "Title",
    Arguments = "--no-build --configuration Release --collect \"Code coverage\"",
    Inputs = new()
    {
        { "ContinueOnError", parameters["myBool"] }
    }
}

but this looks less ideal.

premun commented 1 year ago

Hey, Thanks for opening this issue. I agree that this is a very useful feature and should be supported. Unfortunately, I think it will require quite a bit of work as technically a variable can be put in place of basically anything. I think it might have a solution where we create a wrapper type that could be "T or expression" but I will have to do some experiments to verify it can be done.

I will get back to you soon about this.

mathiasi commented 1 year ago

I was also thinking a wrapper would be needed. Maybe something like Conditioned. I did notice also that the following syntax didn't seem to be supported either:

- task: DotNetCoreCLI@2
  displayName: 'dotnet test'
  inputs:
    command: test
    testRunTitle: "Title"
    projects: |
      *.Test.csproj
    ${{ if eq(parameters.myBool, true) }}:  
      arguments: 'some arguments'
    ${{ if eq(parameters.myBool, false) }}: 
      arguments: 'some other arguments'

Maybe one solution could cover both scenarios

premun commented 1 year ago

Once you're in the Dictionary (like in inputs), this is actually possible:

StepTemplate("template1.yaml", new()
{
    { "some", "value" },
    {
        If.IsPullRequest,
        new TemplateParameters()
        {
            { "pr", true }
        }
    },
    { "other", 123 },
}),

(from https://github.com/sharpliner/sharpliner/blob/main/docs/AzureDevOps/DefinitionReference.md#conditioned-expressions)

mathiasi commented 1 year ago

Ah okay thanks! Though my key reason for using Sharpliner is to avoid dealing with yaml in the first place :D So I thought it could be cool if it was possible in another manner.

premun commented 1 year ago

I agree completely.

I think we will end up wrapping all properties in a Conditioned<>, yeah. For instance, now we have some like that already and some not.

For instance it's possible to do:

{
    Pool = If..
}

But it wouldn't be possible for ContinueOnError. So there is some debt in this where probably everything should be a Conditioned<>. We could then make variable/parameter references (or expressions) implicitly cast into this as well, producing a string ${{ parameter.foo }}.

However, there's one more angle to this - https://github.com/sharpliner/sharpliner/issues/122. This issue would mean wrapping all properties again so that we create a dictionary behind that we serialize. This would mean your C# and YAML would be ordered how you want and not how we decide in Sharpliner. So this is another one where we might potentially have to change dozens of properties. I need to consider both and whether it makes sense to do them together. But your issue has preference 100%. The order is not so important.

premun commented 1 year ago

@mathiasi I was able to make this work using the Conditioned<> everywhere. However, I still think we have some missing spots, e.g. using generic expressions everywhere. Something like this:

    continueOnError: ${{ replace(parameters.someParameter, ' ', '') }}

But I think this should be possible to add with some new Expression() method...

mathiasi commented 1 year ago

That was quick! 😄 Unfortunately I won't have the chance to try it out until a in couple of weeks but I definitely will!