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

Add exist methods for button and select elements #630

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add exist methods for button and select elements #630

wants to merge 1 commit into from

Conversation

grom358
Copy link
Contributor

@grom358 grom358 commented Jan 8, 2015

Added exist methods for button and select elements

@aik099
Copy link
Member

aik099 commented Jan 8, 2015

And I guess the getComputedStyle methods got in this PR by accident again, like they did with your previous PR?

@grom358
Copy link
Contributor Author

grom358 commented Jan 9, 2015

@aik099 gah they did... mmmm.. I think I going to need to delete my Mink repo and refork. Will that cause any issues? will it break the other PR?

I think I must of pushed the getComputedStyle onto my fork's master.

@grom358
Copy link
Contributor Author

grom358 commented Jan 9, 2015

Okay I rebased onto upstream/master to solve for this PR

@aik099
Copy link
Member

aik099 commented Jan 9, 2015

I think I going to need to delete my Mink repo and refork.

I think instead you should be pulling upstream/master into your fork's grom358/origin and then rebasing PR on top of that.

I think I must of pushed the getComputedStyle onto my fork's master.

That's bad news.

Okay I rebased onto upstream/master to solve for this PR

Unfortunately GitHub says otherwise because it can't automatically merge this PR.

@stof
Copy link
Member

stof commented Jan 9, 2015

Okay I rebased onto upstream/master to solve for this PR

I guess you forgot to fetch upstream first, so using an outdated upstream/master

@grom358 grom358 closed this Jan 13, 2015
@grom358 grom358 deleted the webassert branch January 13, 2015 23:03
@grom358 grom358 restored the webassert branch January 13, 2015 23:03
@grom358 grom358 reopened this Jan 13, 2015
@grom358
Copy link
Contributor Author

grom358 commented Jan 13, 2015

My upstream was set to Behat/Mink ... problem solved.

@grom358
Copy link
Contributor Author

grom358 commented Jan 13, 2015

These duplicate scrutinizer errors are bogus.

@stof
Copy link
Member

stof commented Jan 14, 2015

@grom358 we don't consider duplications as failure conditions (because we know it's bogus in several places). the failure condition is for coding styles issues

@stof stof changed the title Webassert Add exist methods for button and select elements Jan 14, 2015
@grom358
Copy link
Contributor Author

grom358 commented Jan 22, 2015

@stof fixed the indenting issues... I had my editor on 2 space indents :S

@grom358
Copy link
Contributor Author

grom358 commented Feb 4, 2015

Was there anything else required for this PR?

@aik099
Copy link
Member

aik099 commented Feb 4, 2015

Maybe squash. @stof , do you see any other changes required before merging?

@stof stof modified the milestone: 1.7 Feb 4, 2015
@grom358
Copy link
Contributor Author

grom358 commented Mar 3, 2015

I have updated this PR for the 1.7 changes. Scrutinizer thinks code is duplicate but its not as far as I can tell. Just number of methods that look similar.

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

Successfully merging this pull request may close these issues.

None yet

3 participants