move the plugin interface into scorch #1507
Conversation
|
NOTE: this will fail until we point to versions with all the other changes merged/tagged, but let's you see the complete picture |
Actually I see now that it won't fail, but will in some intermediate states once the segment.Plugin is removed. Anyway, hopefully you see where this is going. |
index/scorch/segment_plugin.go
Outdated
| var defaultSegmentPlugin segment.Plugin | ||
| // Plugin represents the essential functions required by a package to plug in | ||
| // it's segment implementation | ||
| type Plugin interface { |
There was a problem hiding this comment.
Feels like this name Plugin becomes a very generic name now, without leaving any clues?
Any thoughts?
There was a problem hiding this comment.
Originally I was going make it not exported, but we use it as an argument to the RegisterPlugin method. We could change it to SegmentPlugin. But, then should also change the method names to match?
There was a problem hiding this comment.
Alternatively, if you don't like Plugin at all, maybe SegmentImplementation? That ends up kind of long, and SegmentImpl feels very much like a Java name...
There was a problem hiding this comment.
I liked SegmentPlugin though. .
There was a problem hiding this comment.
OK, I will go with SegmentPlugin, do you think the method names should change as well?
RegisterPlugin -> RegisterSegmentPlugin
ResetPlugins -> ResetSegmentPlugins
and related functions
* update to make all analysis take place in func() by making analysis take place inside a func() a closure can be used to encapsulate all the index specific details. this allows the index API to not know about scorch vs upsidedown analysis structures * rewrite variable declaration for clarity also update comment, per review feedback * update to latest versions selects latest versions compatible with these changes
No description provided.