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

Common platform #317

Closed
wants to merge 1 commit into from
Closed

Conversation

alizelzele
Copy link
Contributor

changing the common mobile platform to support both platforms with only one class make it easier to maintenance + normally the functions that are required for automation are common and dont need any specific driver commands.

@alizelzele
Copy link
Contributor Author

@mach6 the first 3 commits are the pull requests i made. can you check the last commit and give me your opinion on this change?

@mach6
Copy link
Contributor

mach6 commented Oct 4, 2016

@alizelzele I took a look at the last commit in this PR. I have concerns about the approach/changes but don't want to abandon the idea. Let's please focus on getting your other PR's in before proceeding with the "common platform" changes proposed here. Afterwards, we will circle back.

looping in @sebady

@mach6
Copy link
Contributor

mach6 commented Oct 4, 2016

@alizelzele @sebady due to the size of the "common platform" changes, does it make sense to work them on common branch?

@sebady
Copy link

sebady commented Oct 4, 2016

I think we need to segregate the "common platform" commit from the others in this PR.

@mach6
Copy link
Contributor

mach6 commented Oct 26, 2016

@alizelzele can you please follow-up on this now?

@mach6
Copy link
Contributor

mach6 commented Dec 6, 2016

@alizelzele bumping this.. Do you still see these changes as something you want to see merged? if so, will you please update the PR? I'd like to get this closed out.

@alizelzele
Copy link
Contributor Author

@mach6 I completely forgot about this one. will work on it again, and update the code

@mach6
Copy link
Contributor

mach6 commented Dec 13, 2016

@alizelzele Will you please look into the CI failure? It looks like the archetype build is broken due to this change set.

public static SeLionAppiumIOSDriver iOSDriver(){
return SeLionAppiumIOSDriver.class.cast(driver());
}

Copy link
Contributor

Choose a reason for hiding this comment

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

These three methods are appium only. I'd like Grid() to remain neutral w.r.t to web/mobile driver type

@mach6
Copy link
Contributor

mach6 commented Dec 13, 2016

Please update copyright for changes files to include the year 2016

@alizelzele alizelzele force-pushed the common-platform branch 3 times, most recently from a1e9679 to e58ed88 Compare December 20, 2016 07:47
@alizelzele
Copy link
Contributor Author

alizelzele commented Dec 20, 2016

@mach6 Fixed all the issues from checkers, updated copyrights, and move appium driver methods out of Grid. As discussed earlier, this will remove 2 different platform and try to make both of them work as one. That was the reason archytype generator was failing.
It still needs some Updates to remove the old ones completely out of it

1. using only one mobile platform and remove other not required elements
2. extending Abstract Element as base Element to support already defined functionality for Web
3. adding wait for multiple elements in WaitDriver Utils
4. updating tests with the new system
5. updating code generator for new system and removing suport for android and ios directly
6. updating archetype to generate current archetypes based on new changes.
@alizelzele
Copy link
Contributor Author

@mach6 I am closing this one since it is super trouble to rebase it to latest. I'll try my best to make separate smaller pull requests and not to touch core components (as much as possible) :-)

@alizelzele alizelzele closed this Oct 10, 2017
@alizelzele alizelzele deleted the common-platform branch February 27, 2018 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants