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

fix cdi doc missing SpecDB() #96

Merged
merged 1 commit into from
Nov 11, 2022
Merged

fix cdi doc missing SpecDB() #96

merged 1 commit into from
Nov 11, 2022

Conversation

moshe010
Copy link
Contributor

@moshe010 moshe010 commented Nov 9, 2022

Signed-off-by: Moshe Levi [email protected]

@klihub
Copy link
Contributor

klihub commented Nov 11, 2022

LGTM.

That said, I again would like to get rid of the artificial extra registry interfaces (all Registry[A-Z]*). They are neither useful in any way nor correspond to anything in the underlying implementation as all of them are directly implemented by the underlying Cache instance.

Moreover, before ever considering tagging a stable version, I'd like to go back to the more golang-ish very first implementation. There the registry was an implicit/unexported default Cache instance and its full interface was available as package-global functions, operating on that implicit default instance (just like for instance flag is doing with CommandLine). Then operating on the registry would be simply cdi.XyzzyFoobar() instead of needlessly having to go cdi.GetRegistry().XyzzyFoobar(). WDYT, folks ?

@bart0sh
Copy link
Contributor

bart0sh commented Nov 11, 2022

@klihub +1 Getting rid of nested interfaces sounds good to me.

Copy link
Contributor

@bart0sh bart0sh left a comment

Choose a reason for hiding this comment

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

/lgtm

@klihub klihub merged commit 80359a1 into cncf-tags:main Nov 11, 2022
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.

3 participants