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

Introduce description field for function & parameters #137

Merged
merged 2 commits into from Oct 19, 2022

Conversation

dbanck
Copy link
Contributor

@dbanck dbanck commented Oct 14, 2022

This PR introduces a new field, Description, to the functions Spec and Parameter types. I've decoupled this from the actual backfilling of all descriptions, so we and the education team can iterate on the descriptions.

Background

To be able to offer auto-completion of available functions and their signatures within the Terraform language server, we need a longer description for both a function and each parameter.

More information is available in the TF-418: Support Functions in Language Server RFC.

Next Steps

@apparentlymart
Copy link
Collaborator

Hi @dbanck! Thanks for working on this.

This library is a third-party dependency of Terraform rather than actually part of Terraform, and so I considered this proposal from the perspective of cty's own mission rather than anything Terraform might need. Even putting aside Terraform's need to offer a language server, it does seem reasonable to me that any application embedding cty's functions concept will probably benefit from something like a language server that would need this metadata, and so I think it's reasonable to add it.

One specific consequence of having the descriptions embedded directly in the function's definition is that there will be no opportunity to separate the human-readable description of a function from the place where it's implemented. This means that the descriptions returned by the functions that Terraform happens to use will be fixed in this codebase and not customizable by Terraform. Since I'm currently wearing my "cty developer" hat instead of my Terraform hat I'm not going to make a call on whether that's an important consequence or not; I imagine if there is a later requirement for Terraform to use a different description than the one I'd want in this non-HashiCorp library then the Terraform team will find a reasonable technical solution to perform as-needed overrides of the generic descriptions from upstream.

I'm going to merge this now and then I'm going to also merge something like the content of your draft PR because it feels strange to me to release the capability for descriptions without actually including descriptions in the upstream functions. I will probably slightly tweak some of the text you proposed because as you noted those descriptions originate in Terraform's own documentation, which has more specific concerns than this library does when taken alone.

Thanks again!

@apparentlymart apparentlymart merged commit 31869a5 into zclconf:main Oct 19, 2022
@dbanck dbanck deleted the f-introduce-description branch October 25, 2022 08:11
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.

None yet

2 participants