Skip to content

Conversation

@ritro
Copy link
Contributor

@ritro ritro commented Mar 11, 2013

Added comments about exceptions to public methods.

Copy link
Member

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calls the ActionPerfomed(Actions.WindowMessage) which causes the recursion. I see now that I didn't check properly this pull request, this should be in the ActionListener commit. My fault.

@JakeGinnivan
Copy link
Member

There are a few different things in this pull request.

  • Adding comments, I like this and want to pull it in
  • I need to have a think about the IMappableUIItem change. And at a minimum there should be a codebase test which validates that no primary mapping types are missing from White by inspecting the control type dictionary.
  • The interfaces which have been renamed to put the I infront of them. That should be a separate PR and ALL interfaces should be renamed, this change actually makes White less consistent at the moment.

Let me know what you think about this feedback, and where we should go next

@ritro
Copy link
Contributor Author

ritro commented Mar 12, 2013

I see now that this commit was not checked properly by me. It has changes in Window class from ActionListener commit, and shouldn't be here. I think I will clean it first then I will have free time. As for the separating the commit into 2 - I will think what I can do.

@JakeGinnivan
Copy link
Member

If you need git help to cleanup via interactive rebase, add me on Skype jake at ginnivan.net and I can walk you through it

Sent from my Windows Phone


From: ritromailto:notifications@github.com
Sent: ý12/ý03/ý2013 18:19
To: TestStack/Whitemailto:White@noreply.github.com
Cc: Jake Ginnivanmailto:Jake.Ginnivan@readify.net
Subject: Re: [White] Comments to many methods. UIItem is restricted from being a T in generic 'Get'. (#93)

I see now that this commit was not checked properly by me. It has changes in Window class from ActionListener commit, and shouldn't be here. I think I will clean it first then I will have free time. As for the separating the commit into 2 - I will think what I can do.


Reply to this email directly or view it on GitHubhttps://github.com//pull/93#issuecomment-14767649.

@ritro
Copy link
Contributor Author

ritro commented Mar 17, 2013

Only "comments" part is now here.

@JakeGinnivan JakeGinnivan mentioned this pull request Jul 29, 2013
@JakeGinnivan
Copy link
Member

Superseded by #127

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.

2 participants