Often, plugins need to find all keys of a type.
Suggest implementing
func (h *Helpers) KVListAll(options ...Options) ([]string, error) ([]string,
error)
For now there should be two options:
Prefix(prefix string)
WithChecker(f func (key string) (continue bool, err error))
If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.
New contributors please see our Developer's Guide.
I want to help with this.
Thanks @a8uhnf :+1:
@levb What do you think about changing the signature to KVListPrefix(prefix string, page, perPage int)
to be consistent with our other API methods?
@hanzei the whole point of this helper is to enumerate the entire set of keys of a type, without having to bother with the paging. 1/5 paging should be internal to the implementation. An overall limit of items to return should be enough. Maybe KVListAllWithPrefix
?
I also contemplated a KVPrefixDo that would take a function, but decided against it for simplicity.
Often, plugins need to find all keys of a type.
@levb Can you please point me to an example plugin where this is required?
@RajatVaryani Here's a pending PR, this request came out of it. It has 2 examples, grep for KVList, https://github.com/mattermost/mattermost-plugin-jira/pull/351/files#diff-3b02caad0024138fae666f1168b4d0cdR487.
This (first) example performs extra filtering, thus my comment about a KVPrefixDo
being (maybe) a better way to express the functionality. 0/5 maybe implement them both?
@hanzei the whole point of this helper is to enumerate the entire set of keys of a type, without having to bother with the paging. 1/5 paging should be internal to the implementation. An overall limit of items to return should be enough. Maybe
KVListAllWithPrefix
?
I agree and like KVListAllWithPrefix
. What is the use case for a limit?
@hanzei Made me think, I agree, let's simplify to func (h *Helpers) KVListAllWithPrefix(prefix string) (keys []string, err error)
.
Opinion on a supplemental/underlying func (h *Helpers) KVDoWithPrefix(prefix string, func (key string) (continue bool, err error)) error
?
:+1: func (h *Helpers) KVListAllWithPrefix(prefix string) (keys []string, err error)
The name KVDoWithPrefix
confuses me. What does the Do
mean? Is it a KVListAllWithChecker
?
Maybe we want a KVListAll(options ...Options) ([]string, error)
and support various options like Prefix(prefix string)
or WithChecker(f func (key string) (continue bool, err error))
. What do you think?
馃憤 to options in this use-case! KVListWithOptions
FTW.
@a8uhnf Do you have any questions? Is the ticket clear to you?
Hi @a8uhnf, let us know if you're still open to working on this one? Let us know if you have any questions!
In the meantime, I'll open this up for grabs :)
I'd like to help out on this one.
Awesome, thank you @dlclark! Let us know if you have any questions
For this ticket is there an expectation of refactoring the commit mentioned by @levb (https://github.com/mattermost/mattermost-plugin-jira/pull/351/files#diff-3b02caad0024138fae666f1168b4d0cdR487) to use the new functionality?
Left to my own devices I'd think it'd be a separate ticket, but since I'm new here I don't want to assume too much.
It would be great, if you would refactor the code in the jira plugin. But you'r PR alone is great, there is no pressure to do more.
2/5 @dlclark file a new issue https://github.com/mattermost/mattermost-plugin-jira/issues/new, it's a great starter ticket for someone else? And you'll get the pleasure of being notified when someone uses the code you committed? :)
Currently making an attempt to use code from this PR here. So far so good :)
Most helpful comment
Currently making an attempt to use code from this PR here. So far so good :)