search

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

Strong-typed if/inline conditions #238

Closed thomhurst closed 1 year ago

thomhurst commented 1 year ago

IfCondition is what it says on the tin - An if statement in your pipeline e.g.

- ${{ if startsWith(parameters.Param1, 'Param') }}:
  - name: feature
    value: on

InlineCondition is for Stages/Jobs/Steps/Runnable parts of your pipeline e.g.

condition: startsWith('${{ parameters.Param1 }}', 'Param')

Why? Because they're different and can accept different types of values.

If statements are all compile time based. This means they can't evaluate runtime variables in their conditions. Also, parameters won't need to be surrounded with the ${{ param }} syntax.

InlineCondition are executed at runtime. This means they can evaluate runtime variables. Parameters will need the ${{ param }} syntax around them.

Because these are now strongly typed objects, we can enforce passing different values. This means if statements won't accept things like runtime variables.

This PR is quite a change, so let me know what you think.

I also brought in another library called OneOf so that I could bring in different types without creating hundreds of overloads.

thomhurst commented 1 year ago

Think I've fixed the build issues and nullability warnings. Yeah the static variable Vs variable thing does add some complexity for the user. I couldn't think of another way around it though. And I do think it's useful to tell the user that you can't use runtime variables in if conditions. I was trying this the other day and it took me ages to realise why my pipeline wasn't running properly!

premun commented 1 year ago

The pipeline is failing because we're changing the public API of the Sharpliner library (that's the new test I've added). You will have to run the test locally and overwrite the .txt file with the new expected output. We can then review that nothing we didn't want was made public in this change.

I might improve the error message in the test's exception to output these instructions.

premun commented 1 year ago

@thomhurst thanks again for this contribution! I think this is the largest change we've had since tha launch. It's very complex but I think the use of OneOf was a very elegant solution. I also really think that this will bring a lot of value to the users (though they might not even realize it, which makes it even better!).