-
Notifications
You must be signed in to change notification settings - Fork 115
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
Add a guide to writing a composition function in Go #597
Conversation
✅ Deploy Preview for crossplane ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
template Crossplane resources. Crossplane calls composition functions to | ||
determine what resources it should create when you create a composite resource | ||
(XR). Read the | ||
[concepts]({{<ref "../../master/concepts/composition-functions">}}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@plumbis I'd prefer to link to latest, or failing that v1.14 specifically.
I tried a regular [foo](/latest)
style link, but HTML test didn't like it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add docs.crossplane.io/latest/*
to the htmltest config, since this is a deploy-time path.
docs/utils/htmltest/.htmltest.yml
Line 6 in 5dcc5b8
IgnoreURLs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leaving a partial review for now because the commits being pushed to this PR while i'm reviewing caused github to force me to refresh and I lose a comment or two i think - checkpointing to not lose anything else
content/knowledge-base/guides/write-a-composition-function-in-go.md
Outdated
Show resolved
Hide resolved
content/knowledge-base/guides/write-a-composition-function-in-go.md
Outdated
Show resolved
Hide resolved
content/knowledge-base/guides/write-a-composition-function-in-go.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically great @negz!!! a few questions, improvements, and some odd renderings, but basically resolve these as you see fit and ship it!!
content/knowledge-base/guides/write-a-composition-function-in-go.md
Outdated
Show resolved
Hide resolved
content/knowledge-base/guides/write-a-composition-function-in-go.md
Outdated
Show resolved
Hide resolved
Remove the big example, there's now something better in the docs. The linked guide won't exist until the below PR is merged. crossplane/docs#597 Signed-off-by: Nic Cope <[email protected]>
Remove the big example, there's now something better in the docs. The linked guide won't exist until the below PR is merged. crossplane/docs#597 Signed-off-by: Nic Cope <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @negz for the guide, it looks great! I have two things to add:
- I believe it's worth mentioning that functions need to be idempotent. It's an easy mistake to forget that (especially for an operation like create) and act like they're running only once.
- We may add an
examples
folder to the function template and encourage function authors to add examples. Later, we can display them in the marketplace, just like we do with providers.
@ezgidemirel I coincidentally actually did that last night - https://github.com/crossplane/function-template-go. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is something wacky with how vale is building tokens from words and parsing. Testing locally adding the ticks around function-template-go
on line 434 and xpkg.upbound.io
on line 482 made the other spelling errors go away.
Can you try making those changes and see if vale stops grumping?
content/knowledge-base/guides/write-a-composition-function-in-go.md
Outdated
Show resolved
Hide resolved
content/knowledge-base/guides/write-a-composition-function-in-go.md
Outdated
Show resolved
Hide resolved
This is separate from the functions documentation under concepts because: 1. I expect to have similar guides for other languages, such as Python. 2. The guide will evolve outside of the Crossplane release cycle. Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
Vale considers it a spelling mistake, and it's easy to rephrase around. Signed-off-by: Nic Cope <[email protected]>
We don't have any docs specific to multi-platform _packages_ AFAIK. Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
To workaround spellchecker thinking "Go." (with period) is a spelling mistake. Signed-off-by: Nic Cope <[email protected]>
I do actually want latest, but HTML test complains when I try. There's no v1.14 release directory to link to yet, so master will have to do. Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
Signed-off-by: Nic Cope <[email protected]>
@plumbis It worked, thank you! |
@ezgidemirel Agreed - I'll do a pass to make sure this is really clear. I'd like to get this merged as-is though since we shipped today. Will do that in a follow-up. |
This seems like the most future-proof option for this guide, which lives outside of any version. Signed-off-by: Nic Cope <[email protected]>
Fixes #579
This is separate from the functions documentation under concepts because: