-
Notifications
You must be signed in to change notification settings - Fork 93
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
doc: Add a tutorial how to write plugins #1034
Conversation
a76e721
to
a24e098
Compare
cf42008
to
731b72b
Compare
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.
I only have a small review on the code. Then LGTM
constexpr const char * attrs[]{"author.name", "author.email", "description", nullptr}; | ||
constexpr const char * attrs_value[]{"Fred Fedora", "[email protected]", "Plugin description."}; | ||
|
||
class TemplateCmdPlugin : public IPlugin { |
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.
I would suggest to add a comment for the class and each of its methods, including inheritance.
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.
May I ask you to add class documentation for class IPlugin
?
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.
I've provided some additional brief docstrings for the IPlugin
classes in dnf5 and libdnf5 namespaces. Please let me know if more detailed information would be appropriate.
[main] | ||
name = template | ||
enabled = yes | ||
version = 1.0.1 |
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.
Is configuration file mandatory? Is any of the keys mandatory? How the version key is used? May be I overlooked something and it is described in documentation somewhere.
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.
I've tried to make it more explicit in the docs text and I've also added a comment into the example configuration file.
And some final docs fixups.
731b72b
to
0cca56f
Compare
I've tried to address all comments. Please let me know if I've missed anything. |
LGTM |
Documentation updates cover the following aspects:
Closes #287.