Changed the `$CONST` keyword into `$CONSTANTS` for ubiquity purpose

[cdavernas]

trafficstars

Many thanks for submitting your Pull Request :heart:!

Please specify parts this PR updates:

• [x] Specification
• [ ] Schema
• [ ] Examples
• [ ] Extensions
• [ ] Roadmap
• [ ] Use Cases
• [ ] Community
• [ ] TCK
• [ ] Other

What this PR does / why we need it:

Changes the $CONST keyword into $CONSTANTS for ubiquity purpose

[ricardozanini]

That's fine, but it will break compatibility with the previous versions.

[cdavernas]

@ricardozanini indeed, that's why I wanted to squeeze it in before 0.9 which, as far as I'm aware of, not gave any of thoses

[cdavernas]

@fjtirado Replace all and that's done. Not such a huge compatibility problem imho. Plus, I don't see why you couldn't support both keywords before 1.0 if you don't want to go down that path.

The need for that change imo is ubiquity. The DSL is to be used by non-technical profiles too, and using abbreviations is not recommended, not even in dev, in code. Ubiquity guarantees intuitive understanding of what you are looking at.

CONST could for example stand for constraints, which us a term that will definitely kick it at some point. Imho, it's not accurate enough.

Otherwise why not rename WORKFLOW to WF or SECRETS to SEC too then, if shortness is the goal. CONSTANTS is just one letter longer than WORKFLOW. We could even use CONSTANT if you prefer.

[ricardozanini]

To be fair, const is a very well-known abbreviation for constants.

[cdavernas]

@ricardozanini I agree totally, not saying it's obscure terminology or whatever, but is less reader friendly imo, and less ubiquitous: it could stand for constraints too, for instance

[cdavernas]

The other terms we have been using are on point (workflow, secrets, etc), perfectly explicit. Not that one, imo.

[fjtirado]

@cdavernas Implementing $CONST in jsonpath properly was not straighforward. Although that is a proper language construct for a variable in jq, the proper language construct for jsonpath will be @.CONSTANT (the same applies for secret and workflow) If you really want to have a valid expression without doing hacky string replacement ( I think we agree we should not evaluate $CONSTANT/CONST in JQ is in between quotes) before evaluating it, I would argue that the spec should just define the magic word (WORKFLOW, CONST/CONSTANT, SECRET) and states that this should be variables of the expression. And still not able to see what's the benefit of changing that, const is intuitive shortcut for constants, as C++ language demonstrated Also, another complaint is of procedure: why there was no issue to discuss this breaking compatibility change before actually opening the PR?

[cdavernas]

Also, another complaint is of procedure: why there was no issue to discuss this breaking compatibility change before actually opening the PR?

@fjtirado We have been doing PR, then discussing on them for a while now, but ok. I don't think we have established any "procedure" as you say, especially if said PR concerns a 1mn "Replace All" kind of work. I guess, however, that you are right: we should probably start with a GitHub conversation rather than with a PR.

Implementing $CONST in jsonpath properly was not straighforward

What do you mean by JsonPath? Are you using it for expression evaluation? With jq - which is the default expression language-, you can pass json arguments like, say: --argjson CONST {CONSTJSON} so in my use case it's an easy fix, really.

And still not able to see what's the benefit of changing that, const is intuitive shortcut for constants, as C++ language demonstrated

C++, C#, whatever, I agree with you, as I said to @ricardozanini, I'm not contradicting the fact that in our world (tech one, it is), const is a perfectly ubiquitous terminology; I'm just saying it might not be the case for a person working in, say business, and wants to use SW to orchestrate high level business tools. Furthermore, I believe that the idea that a concept such as "constraints" might come into play at some point in time (spec or runtime side) is reasonable, which would IMO make the term confusing.

Please understand I've no other interest than ubiquity here, it implies as well a (minor) change on Synapse, for example. So if you think we should not do that, and that's the consensus, then I'll close the PR 😉

But I'd like to hear what @mswiderski, @JBBianchi, @antmendoza and @tsurdilo think about it before taking it one way or the other (or another 😜)

[fjtirado]

Regarding jsonpath, as far as I know, is still supported by the spec (although fortunately is not the default one anymore), but my point is that a user of the spec might want to add another expression language where the way to refer to a variable is not $<varName> but just #<varName>, so it might be reasonable for him to want to use #CONSTANT rather than $CONSTANT when writing the expression. Anyway, thats a different issue not related with the reserved word name.

[cdavernas]

Oh ok! Yeah the jq vs jsonpath vs ... is yet another discussion, but yeah, of course, any and all implementations are/should be allowed. I really never proposed to force the use of the $ prefix or anything like that.

[fjtirado]

Oh ok! Yeah the jq vs jsonpath vs ... is yet another discussion, but yeah, of course, any and all implementations are/should be allowed. I really never intended to force the use of the $ prefix or anything like that.

Implementation wise to support constant rather than const just implies changing the value of a constant (no pump intended ;)) but to support both, implies changing the expression evaluator to add a new constant that is resolved the same way than the previous one. This implies at least three classes (the two languages expression implementations plus the one holding the constant) rather than one. Anyway, not a big deal, so if this goes forward, Ill probably do that to avoid changing all examples that currently use const. But I think we need to define a procedure that, for every breaking compatibility change, albeit minimum one, we should open an issue to discuss first. In this particular case, it is just a question of taste, I prefer the existing word than the proposed one (const/constant refers to the same concept, there is not ambiguity when using the shorter one). And I also feel that when changing something that is already there, it needs to be a good reason to do that (the existing one is either wrong, ambiguous, error prompt or any other reason I cannot foresee in this case).

[tsurdilo]

We will make updates for 0.9 for error handling and top-level start def which will be much bigger breaking changes from 0.8, but would really like to only then have non-breaking stuff from 0.9 to 1.0 (wont say 100% in case we find some mayor issues that need to be addressed).

For this change, think its fine to support both ways if we feel its necessary for some reason. We could also deprecate one and intro the other and then remove the one thats deprecated in 1.0. Really up to you guys I'm ok with this either way.

[cdavernas]

@fjtirado @tsurdilo @ricardozanini Should we proceed with that PR, or just drop it alltogether? Maybe we should put it up for a vote?

[tsurdilo]

@cdavernas unless this is urgent for you for some reason i'd say lets stash it and revise later if possible

[cdavernas]

@tsurdilo Not urgent at all, no. But I'd be happier tackling this, if ever, sooner than later, as it is a breaking change

[ricardozanini]

I think we are missing a governance model to handle this kind of situation, like voting in a (sort of) controversial feature.

[cdavernas]

Agreed, let's then vote on that one on next meeting.

Created #644 to address future situations

[tsurdilo]

ok ill update the website and readmes today to reflect the meeting schedule we brought back to life :)

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.