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

doc new features #136

Closed
wants to merge 1 commit into from
Closed

doc new features #136

wants to merge 1 commit into from

Conversation

eguzki
Copy link
Contributor

@eguzki eguzki commented Nov 7, 2024

  • String extension funcs
  • Query string decoding queryMap
  • getHostProperty

Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
@eguzki eguzki added the documentation Improvements or additions to documentation label Nov 7, 2024
@eguzki eguzki requested a review from alexsnaps November 7, 2024 14:29
@eguzki eguzki self-assigned this Nov 7, 2024
@alexsnaps
Copy link
Member

I really don't understand this! This is "shoveling snow forward" as we say in French Canada:

pelleter par en avant

We're literally creating technical documentation debt for ourselves in my opinion. So my questions are:

  • Who are you writing that documentation for?
  • What do you think you're achieving by adding this? And doing this in this very way?

I have opinion on both of these, but I am really trying to understand where you are coming from?

@eguzki
Copy link
Contributor Author

eguzki commented Nov 7, 2024

Documenting exposed functions and capabilities for anyone writing wasm configuration and CEL predicates

@eguzki eguzki closed this Nov 7, 2024
@alexsnaps
Copy link
Member

Documenting exposed functions and capabilities for anyone writing wasm configuration and CEL predicates

Right! The former is... interesting, but I think the important part is the latter!

capabilities for anyone writing wasm configuration

in that category, I would argue that queryMap falls into. And for now, at least in my opinion, only kuadrant-operator writes wasm configurations. Which is why I documented queryMap the way that I did, i.e. in the code based on your request.

functions and capabilities for anyone CEL predicates

That bit I think matters most! That's not a wasm-shim thing per se tho. Or certainly not the only piece of our stack owning that responsibility. So, yes! Let's document this... somehow (and I really don't know where or how tbh). So who writes CEL expressions (predicates or not): Kuadrant Policy authors and... Authorino AuthConfig authors. For the former, I think that exposing the existence and semantic of "functions and capabilities" is certainly important. But how would we best do that? Does that documentation exist already? As CEL itself should be documented, then the extensions (whether standard or not) should also be. Can we link to existing stuff? Whether yes or no, where do we want that documentation to be available to our users? Finally which are the things that we want to doc for these users? Not queryMap and ... I personally asked that we do not document getHostProperty to our users. But if there is a reason to, I'm open to hearing that.

So I see you closed this... but possibly the String extension should be committed somewhere to enrich our users docs. But I really think here is not the place.

@alexsnaps
Copy link
Member

The best I could find wrt the Strings extension is from the README.md on the cel-go implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants