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

Use either expr or govaluate consistently, not both #6823

Closed
scravy opened this issue Sep 29, 2021 · 6 comments
Closed

Use either expr or govaluate consistently, not both #6823

scravy opened this issue Sep 29, 2021 · 6 comments
Labels
area/spec Changes to the workflow specification. area/templating Templating with `{{...}}` solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) type/feature Feature request

Comments

@scravy
Copy link
Contributor

scravy commented Sep 29, 2021

Summary

Either when: should use antonmedv/expr or depends: and {{# template tags should use Knetic/govaluate.

There should be just one expression language/syntax be used consistently. Personally I'd be in favor of expr, but that matters less to me than to have a consistent developer experience.

Use Cases

when:, depends: and {{# expr template tags. Currently these use different expression syntax, when: uses govaluate, whereas depends: and {{# uses expr. This is confusing and not very maintainable.


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

@scravy scravy added the type/feature Feature request label Sep 29, 2021
@alexec
Copy link
Contributor

alexec commented Oct 1, 2021

I agree with this, but i don't know how we'd remove support without breaking users workflows.

@scravy
Copy link
Contributor Author

scravy commented Oct 3, 2021

Are there any usages of govaluate other than when:? Maybe when: could be deprecated (without removal) and an if: could be introduced which uses antonmedv/expr. This way user workflows using when: with govaluate expressions should continue to work.

@alexec
Copy link
Contributor

alexec commented Oct 4, 2021

I think a new if is a great idea.

@denis-codefresh
Copy link
Contributor

@alexec I would like to work on it. Seems like a good first issue. Could you assign it to me?

@alexec
Copy link
Contributor

alexec commented Oct 27, 2021

@denis-codefresh I think we (i.e. you) should create a new issue for if. We should get @jessesuen to give comment too.

@agilgur5
Copy link

Going to close this out as duplicative of #7831 since the decision has been made with multiple maintainers accepting and #7831 is a specific proposal to address it.

@agilgur5 agilgur5 added the solution/duplicate This issue or PR is a duplicate of an existing one label Sep 21, 2023
@agilgur5 agilgur5 added solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) and removed hacktoberfest solution/duplicate This issue or PR is a duplicate of an existing one labels Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/spec Changes to the workflow specification. area/templating Templating with `{{...}}` solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) type/feature Feature request
Projects
None yet
Development

No branches or pull requests

4 participants