-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[py] websocket-client v.1.8.0 was added to setup.py #14187
[py] websocket-client v.1.8.0 was added to setup.py #14187
Conversation
PR Reviewer Guide 🔍
|
PR Code Suggestions ✨
|
@titusfortner I also noticed that the build script |
I agree they should all probably match. I don't know how python does things. In ruby if it is a transitive dependency, we leave the requirements loose to give the user more flexibility. I'm not sure if that matters here. |
It depends on the build system. With
The more loose requirements there are, the more flexibility the build system has to assemble a "working" configuration without conflicts. Managing a strictly defined set of dependencies that are tested thoroughly during build and testing phases makes configuration management easier. Updating dependencies becomes a more manageable process. Potentially, we might encounter version conflicts if other dependencies require |
Python has had a lot of different fingers in the pot over the years, it is not surprising that there are things that have been missed as different people add things. Yes, please, update the things that make sense. Thanks. |
@titusfortner I made changes to match dependencies in setup.py and BUILD.bazel. I think we should freeze dependencies to specific versions separately if needed later on. Right now, there are differences in library versions: for instance, |
What do you suggest? I wish I knew more about bazel python rules in general. |
@titusfortner I propose to accept the current pull request as is. Improvements for the build and dependencies will be considered later in a separate task. I also need to delve deeper into Bazel build rules |
@iampopovich do these happen as part of your tooling or work flow? I can't see the results of the tests whenever an update is made 😁 |
@titusfortner I updated the branch to pull in the latest changes from trunk. If it's not necessary, I'll update the branch locally only. |
GitHub will automatically squash & rebase for us, so if there aren't any changes that are likely to cause an issue, it isn't needed to keep merging trunk. It's only an issue because I want to merge it and it shows tests haven't run. 😁 Thanks for the code! |
* websocket-client v.1.8.0 was added to setup.py * update for py/setup.py * added dependencies with the same versions in both BUILD.bazel and setup.py * added supported python versions to BUILD.bazel
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
according to request in #14184 websocket-client was added to setup.py
version of client the same as in py/requirements.txt
Motivation and Context
Types of changes
Checklist
PR Type
Bug fix, Dependencies
Description
websocket-client
version 1.8.0 to theinstall_requires
list insetup.py
.Changes walkthrough 📝
setup.py
Add websocket-client dependency to setup.py
py/setup.py
websocket-client
version 1.8.0 to theinstall_requires
list.