Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
cached plugin enum #768
cached plugin enum #768
Changes from all commits
4c388f5
9c2cdfc
6cf8152
ff87bac
63de9cc
76c94f0
33c6f9c
b6e8381
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
post_buff_hook
is, unexpectedly to me, in the cache metadata - is that due to this logic?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 existing
info
includedpost_buff_hook
:main
We can further refine the
info
data to be provided in future iterations.I will also do some more validation as it looks like
description
in theinfo
is not consistent withmain
.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.
Ah nice. _post_buff_hook should def not be there. Some plugins change their descriptions during the constructor iirc.
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.
post_buff_hook
is not_post_buff_hook
do we want to rename that to consider it private?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.
It looks like
description
is added in the base class constructor if not already provided in the class implementation for most of the plugins, this results in an inconsistency since the cache build actually avoids calling the constructors in this PR.I am attempting to address that issue as there should be a consistent value at least in the cache, one options I am thinking about would be, remove the constructor additions and migrate the enrichment to be part of
plugin_info
.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.
would like to see default params included here also. maybe something like this
this makes some assumptions about default param names not conflicting with class attributes, but i hope testing elsewhere covers that
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.
This might be something to consider doing in the display logic, not sure it needs to be in the json cache file in a specific format.