-
-
Notifications
You must be signed in to change notification settings - Fork 78
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 functions to Admin API and there response type #181
Conversation
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.
In general totally fine with adding new admin API wrappers, but the code quality needs a bit of improvement. Some of the comments apply to more than one place (I didn't want to repeat the same comment multiple times), so please fix all similar points
Thanks for reviewing, going to fix it |
Hi @tulir , |
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.
Looking better, but some fields still need better types and the Messages
function has a duplicate. Also check the lint errors that GitHub actions reported (or run pre-commit run -a
locally)
Thanks for reviewing @tulir ! |
Ah yeah running pre-commit with |
need more changes or all good? |
As Tulir mentioned, the linter must pass. |
my local config was missing goimport ^^ |
Hi !!
I'm using your lib to implement a benthos input for synapse and i needed more Admin API so i implemented it in this pull request.
Tell me if it's ok for you to add theses changes.
Have a nice day,
++