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

Add enumerate methods to the multicodec registries. #176

Merged
merged 1 commit into from
May 25, 2021

Conversation

warpfork
Copy link
Collaborator

(This is a logical port of #169 to follow #172 .)

I've added more documentation -- particularly, cautionary notes on the package-scope functions which read the global shared state.

(This is a logical port of
#169 to follow
#172 .)

I've added more documentation -- particularly, cautionary notes on the
package-scope functions which read the global shared state.
@warpfork
Copy link
Collaborator Author

For what it's worth, still I don't think this is a particularly good idea. I don't like globals in general (and this patch and the discussion around it makes me think we may have already gone to far); and in this case, I also don't think this abstraction is featureful enough to be a good coordination point for applications which have plugin systems, and don't really want to encourage it to be used for that purpose (and I don't know what else this would be used for). But, I'm considering myself outvoted, so, here we go 🤷

In the event this does cause problems in the future... part of the reason I'm willing to roll with this is there's also a Ulysses Compact I'm going to deploy on myself for this, and @aschmahmann and @willscott agreed to find tolerable:

  • We are adding these listing functions to the global registry (despite the concern this may encourage people to put Logic around this global in ways that make them and us sad in the future).
  • We also introduce the multicodec.Registry type, as already PR'd, and also adding these listing functions to that type. (The multicodec.Registry type already has disclaimers and warnings about how any mutations are something you need to synchronize yourself as necessary.)
  • The first time anyone reports any kind of bug in this area and I see it relates to an attempt at building Logic around this global, then I reserve the right to immediately, pushing directly to master, without delay, completely remove these listing APIs from the global.
  • In that case, I will not remove the listing functions from the multicodec.Registry type. This will leave a smooth path forward to whoever needs it.

@warpfork warpfork changed the base branch from multicodec-registry-type to master May 25, 2021 00:17
@warpfork warpfork merged commit b39b96a into master May 25, 2021
@warpfork warpfork deleted the multicodec-registry-listing branch May 25, 2021 00:18
@aschmahmann aschmahmann mentioned this pull request Aug 23, 2021
62 tasks
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.

2 participants