-
Notifications
You must be signed in to change notification settings - Fork 41
fix(mosquitto): module name #97
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
base: main
Are you sure you want to change the base?
fix(mosquitto): module name #97
Conversation
Could you resolve conflicts 🙏 ? Then we can proceed with the merge |
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.
Left a few comments that need to be resolved before accepting these changes
@@ -4,7 +4,7 @@ categories: | |||
- message-broker | |||
docs: | |||
- id: python | |||
url: https://testcontainers-python.readthedocs.io/en/latest/modules/mqtt/README.html | |||
url: https://testcontainers-python.readthedocs.io/en/latest/modules/mosquitto/README.html |
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 this URL is correct, so I'm not sure of the validity of this PR.
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.
testcontainers/testcontainers-python#740 suggests to rename the module. If so, then this change applies.
@@ -13,7 +13,7 @@ docs: | |||
``` | |||
installation: | | |||
```bash | |||
pip install testcontainers[mqtt] | |||
pip install testcontainers[mosquitto] |
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.
question: could you point me to the pip link to install the package so I can verify if this change is also needed? I was not able to locate it myself 😞
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.
the "extra" which is mqtt
today and is being requested to be changed to mosquitto
in the docs above is in fact listed here - https://github.com/testcontainers/testcontainers-python/blob/9317736c34cbe23844006c8e49629fc88e142949/pyproject.toml#L146
There is typos in documentation and problem with naming convention of mosquitto module in testcontainers-python.
I have prepared RP to fix name testcontainers/testcontainers-python#740
Once it will be merged, documentation should be updated
Thanks!