-
Notifications
You must be signed in to change notification settings - Fork 568
chore: remove unused commands #1132
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
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.
Pull Request Overview
This PR removes deprecated and unused touch action commands from the WebDriver implementation.
- Deleted MJSONWP
TOUCH_ACTION
andMULTI_ACTION
registrations in the command executor. - Removed the corresponding
TOUCH_ACTION
andMULTI_ACTION
constants fromMobileCommand
.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
appium/webdriver/webdriver.py | Removed registration of deprecated TOUCH_ACTION and MULTI_ACTION commands. |
appium/webdriver/mobilecommand.py | Deleted unused TOUCH_ACTION and MULTI_ACTION constants. |
Comments suppressed due to low confidence (2)
appium/webdriver/webdriver.py:481
- The FIXME comment about MJSONWP commands is now outdated since these commands have been removed; please remove or update this comment accordingly.
# FIXME: remove after a while as MJSONWP
appium/webdriver/mobilecommand.py:32
- After removing these commands, add or update tests to ensure that attempts to use
MobileCommand.TOUCH_ACTION
andMULTI_ACTION
are handled or rejected appropriately.
TOUCH_ACTION = 'touchAction'
@@ -30,9 +30,6 @@ class MobileCommand: | |||
GET_CURRENT_CONTEXT = 'getCurrentContext' | |||
SWITCH_TO_CONTEXT = 'switchToContext' | |||
|
|||
TOUCH_ACTION = 'touchAction' |
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 is a breaking change. We need to bump the major client version in order to remove deprecated endpoints
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.
Do you mean for users who implement this endpoint by themselves by referencing this constant?
This client's implementation for this has been removed in the Python client v3 -> v4, so these are leftover then.
Then, I'll bump this major version after this 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.
I did not know the client implementation has been already removed. Then it's probably fine without bumping the major
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.
Ah, gotcha
They have already been removed.