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

Update intro examples #177

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

TristonianJones
Copy link
Collaborator

Ensure the intro examples are consistent with the latest function signatures.

@@ -30,8 +30,8 @@ example, given an instance of the `Account` type assigned to the variable

```proto
has(account.user_id) || has(account.gaia_id) // true if either one is set
size(account.emails) > 0 // true if emails is non-empty
matches(account.phone_number, "[0-9-]+") // true if number matches regexp
account.emails.size() > 0 // true if emails is non-empty
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the 4 receiver-style size() declarations to the list of standard definitions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've basically replaced the global existing definitions with the receiver-style ones. That's been the preference internally for quite some time, but I didn't realize this wasn't reflected publicly.

@@ -942,31 +942,31 @@ specified by the following overloads:
size
</th>
<td>
(string) -> int
string.() -> int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm okay marking the function-style version as "deprecated", but I don't think it's okay to just remove it. Anyone attempting to implement a compatible CEL implementation will need to provide the function-style versions for the foreseeable future, so it ought to be in the language spec.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, now it's a confusing example because we're not listing the full set of overloads. Unfortunately, the other functions with multiple overloads are either:

  • Operators, which introduce an unnecessary complication.
  • Conversion functions, which we single out in their description as being different.
  • The get*() functions on timestamps and durations, but we're going to deprecate them on the durations.
  • Other oddballs like dyn().

So consider whether there's a better example to use here, but I'm okay with just the receiver-style size functions if you don't think there's a better alternative.

@TristonianJones
Copy link
Collaborator Author

PTAL

@@ -942,31 +942,31 @@ specified by the following overloads:
size
</th>
<td>
(string) -> int
string.() -> int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, now it's a confusing example because we're not listing the full set of overloads. Unfortunately, the other functions with multiple overloads are either:

  • Operators, which introduce an unnecessary complication.
  • Conversion functions, which we single out in their description as being different.
  • The get*() functions on timestamps and durations, but we're going to deprecate them on the durations.
  • Other oddballs like dyn().

So consider whether there's a better example to use here, but I'm okay with just the receiver-style size functions if you don't think there's a better alternative.

number of unicode codepoints in the string
</td>
</tr>
<tr>
<td>
(string) -> int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mark the function-style overload as "deprecated", here and below. (Boy, I wish we had adapted Uniform Function Call Syntax and we wouldn't have had to worry about any of this.)

Copy link

@BronkHonkyTonk BronkHonkyTonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc/intro.md

@BronkHonkyTonk
Copy link

Leave a ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants