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.

Sign up for GitHub

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

Jump to bottom

Extend motor option to beanie #995

Merged
merged 3 commits into from
Sep 24, 2024
Merged

Extend motor option to beanie #995

merged 3 commits into from
Sep 24, 2024

Conversation

Dudesons
Copy link
Contributor

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
View reviewed changes
@Dudesons
Copy link
Contributor 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...

adeelsohailahmed
adeelsohailahmed reviewed Aug 28, 2024
View reviewed changes
Copy link
Member

@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
Contributor 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
Member

adeelsohailahmed commented Aug 29, 2024 •
edited
Loading

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
Contributor 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

adeelsohailahmed
adeelsohailahmed approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@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
Riverfount
Riverfount approved these changes Sep 4, 2024
View reviewed changes
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

@07pepa 07pepa merged commit 6eb1dd7 into BeanieODM:main Sep 24, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adeelsohailahmed adeelsohailahmed adeelsohailahmed approved these changes

@Riverfount Riverfount Riverfount approved these changes

@07pepa 07pepa 07pepa approved these changes

@LawMixer LawMixer Awaiting requested review from LawMixer

@ayusshsoarav ayusshsoarav Awaiting requested review from ayusshsoarav

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Dudesons @07pepa @adeelsohailahmed @Riverfount