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

refactor: Isolate base Android functionality #901

Merged
merged 24 commits into from
Jan 24, 2024

Conversation

mykola-mokhnach
Copy link
Contributor

@mykola-mokhnach mykola-mokhnach commented Jan 18, 2024

BREAKING CHANGE: Removed androidHelpers export. Corresponding methods are now integrated into the driver
BREAKING CHANGE: Removed webviewHelpers export. Corresponding methods are now integrated into the driver
BREAKING CHANGE: Removed redundant exports related to io.appium.settings.
BREAKING CHANGE: Removed deprecated APIs related to launchApp/closeApp/reset
BREAKING CHANGE: Removed the unused unlocker property from the driver.

The mixin typesystem has been aligned. There are no explicit mixin definitions, which basically duplicate each method signature. Instead all types are now provided next to method definitions.

Improved grouping of different commands. Made it more explicit which of them are designed for export and which are just internal helpers

Improved helper methods design, so they don't need to retrieve driver-specific properties (like log or adb) via command line args, but use this context and explicit binding instead.

Marked legacy JWP touch-related methods as deprecated. They must be eventually replaced with proper W3C implementations.

Removed unused declarations for non-implemented methods.

@mykola-mokhnach
Copy link
Contributor Author

Unit tests obviously fail for now and need an update. Only the codebase has been updated to a buildable state.

@KazuCocoa
Copy link
Member

Took a look entirely. I don't have any objections to this

Copy link
Member

@jlipps jlipps left a comment

Choose a reason for hiding this comment

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

didn't review the code in detail but the big picture changes are OK to me. good to go when tests are in order.

lib/commands/app-management.js Outdated Show resolved Hide resolved
@mykola-mokhnach mykola-mokhnach marked this pull request as ready for review January 23, 2024 20:54
@mykola-mokhnach mykola-mokhnach merged commit 70e6233 into appium:master Jan 24, 2024
6 checks passed
@mykola-mokhnach mykola-mokhnach deleted the refactor_mixins branch January 24, 2024 05:34
github-actions bot pushed a commit that referenced this pull request Jan 24, 2024
## [8.0.0](v7.8.3...v8.0.0) (2024-01-24)

### ⚠ BREAKING CHANGES

* Removed androidHelpers export. Corresponding methods are now integrated into the driver
* Removed webviewHelpers export. Corresponding methods are now integrated into the driver
* Removed redundant exports related to io.appium.settings.
* Removed deprecated APIs related to launchApp/closeApp/reset
* Removed the unused unlocker property from the driver.

The mixin typesystem has been aligned. There are no explicit mixin definitions, which basically duplicate each method signature. Instead all types are now provided next to method definitions.

Improved grouping of different commands. Made it more explicit which of them are designed for export and which are just internal helpers

Improved helper methods design, so they don't need to retrieve driver-specific properties (like log or adb) via command line args, but use this context and explicit binding instead.

Marked legacy JWP touch-related methods as deprecated. They must be eventually replaced with proper W3C implementations.

Removed unused declarations for non-implemented methods.

### Code Refactoring

* Isolate base Android functionality ([#901](#901)) ([70e6233](70e6233))
Copy link

🎉 This PR is included in version 8.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants