-
Notifications
You must be signed in to change notification settings - Fork 13
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 docs and middleware for dynoid #204
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.
This looks really good.
I do have one reservation about putting the middleware under the hmiddleware
package even though it seems like the best fit. This module has quite a few dependencies and I think it would be great if we could eventually break it up a bit.
The dyno ID bits will need to remain together so may consider moving it to dynoid/dynoidmiddleware
instead? That also prevents two packages that are named identically for import purposes.
Co-authored-by: Troels Thomsen <19824+tt@users.noreply.github.com>
Co-authored-by: Troels Thomsen <19824+tt@users.noreply.github.com>
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 think a readme.md
with some basics on how to use this package would be really helpful. That said, the inline comments definitely help, as does having the tests nearby to use as an example.
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.
A few suggestions but still looks good.
Co-authored-by: Troels Thomsen <19824+tt@users.noreply.github.com>
Thanks for the suggestions @tt. I've incorporated your feedback. |
I agree with you @mjrossi, but I figured I would hold off on that until we some devcenter articles and a blog post to link to. |
Rationale
Dyno identity should be easy to adopt.
Changes
dynoid
anddynoid/dynoidtest
.dynoid/middleware
to simplify incorporating dynoid.Meta
Work Item