-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Document "Functional Composition" pattern used in core interfaces #13263
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #13263 +/- ##
==========================================
+ Coverage 91.57% 91.61% +0.03%
==========================================
Files 522 522
Lines 29089 29168 +79
==========================================
+ Hits 26639 26723 +84
+ Misses 1933 1926 -7
- Partials 517 519 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dafa6a3
to
b9fc801
Compare
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.
Having this documented, clearly and explicitly with examples, will be extremely useful for future developers who may be less experienced with 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'm not sure the changelog entry is needed here, since this documents what we're already doing. There's no user-facing change.
Changelog removed. Note that section 5 is somewhat new, the recommendation to provide "Self()" methods. I find it helps remove a number of inline functions returning constant values. |
As a key requirement, every function must have a simple "no-op" | ||
implementation corresponding with the zero value of the | ||
`<Method>Func`. The expression `New<Type>(nil, nil, ...)` is the empty | ||
implementation for each type. |
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.
The no-op is true, but to be able to extend the New<Type>(nil, nil, ...)
we decided to split the functions into required and optional (which by the definition of the interface is fine to be no-op in lots of cases), and use an optional pattern. This may not be the best tradeoff, but allows us to extend the optional functions and not the required, which is ok since any new func/method added to any interface needs to have a default implementation.
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.
Got it. I had described (as you saw) the potential to start with a non-option-taking constructor and add a New<Type>WithOptions
later.! I believe you are saying that constructors should or must support the functional option pattern even when there are no options, from the start. This allows adding options in the future, always without new constructors.
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.
believe you are saying that constructors should or must support the functional option pattern even when there are no options, from the start.
Correct
Description
Documents the interface pattern used in core interfaces (e.g., component, consumer, extensions), which enables safe interface evolution as described in this RFC. This documents an existing pattern so that it can be applied more consistently across our code base.
Testing Documentation
This interface pattern has been checked and documented using examples from PR #13241.