Skip to content
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

Extend motor option to beanie #995

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Dudesons
Copy link

Add the possibility to install optional packages from motor.
As an example using the the aws option allow to connect with an IAM role.

@07pepa 07pepa self-requested a review August 14, 2024 19:42
07pepa
07pepa previously approved these changes Aug 14, 2024
@Dudesons
Copy link
Author

Any chance to have more reviews or other reviewers @07pepa ? or should I go to discord in order to discuss about this PR ?

@07pepa
Copy link
Member

07pepa commented Aug 27, 2024

@Dudesons well i asked on discord and you are welcomed on discord

but i requested usual active reviewers already...

Copy link
Contributor

@adeelsohailahmed adeelsohailahmed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. Could you update the documentation on how to utilize this, please?

@Dudesons
Copy link
Author

@adeelsohailahmed ok, I just added it in the getting_started but not sure if it's the best place or should I put it in the README or both

@adeelsohailahmed
Copy link
Contributor

adeelsohailahmed commented Aug 29, 2024

Thank you for adding the documentation.

@adeelsohailahmed ok, I just added it in the getting_started but not sure if it's the best place or should I put it in the README or both

getting_started seems right to me. Might not be a bad idea to include a condensed version in the README too. For condensed version, we could mention the supported dependencies, and provide an example or two.

@Dudesons
Copy link
Author

Thank you for adding the documentation.

@adeelsohailahmed ok, I just added it in the getting_started but not sure if it's the best place or should I put it in the README or both

getting_started seems right to me. Might not be a bad idea to include a condensed version in the README too. For condensed version, we could mention the supported dependencies, and provide an example or two.

I added the reference from README to getting started

Copy link
Contributor

@adeelsohailahmed adeelsohailahmed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thank you.

@adeelsohailahmed adeelsohailahmed requested a review from a team August 29, 2024 23:52
@Dudesons Dudesons requested a review from 07pepa September 1, 2024 10:22
Copy link

@Riverfount Riverfount left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants