-
Notifications
You must be signed in to change notification settings - Fork 33
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
Added SpyInstanceProvider #24
Conversation
ArcBees » Jukito #69 FAILURE |
Thank you! Please use the same formatting as the rest of the project. We use 4 spaces |
@@ -99,7 +99,7 @@ protected void bindScopes() { | |||
protected <T> ScopedBindingBuilder bindSpy(Class<T> klass) { | |||
return bindNewSpyProvider(Key.get(klass)); | |||
} | |||
|
|||
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.
Empty line should be empty ;)
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 should have run Eclipse's formatter over the whole file but I didn't want to risk breaking other formatting conventions, especially relating to line wrapping.
Yes I realized that afterwards, I fixed some of them, I must have missed some. |
* @param instance The instance to be returned. This instance should be | ||
* immutable; if it is not, you risk polluting your tests | ||
* as the underlying instance is the same (even though it | ||
* uses a different spy wrapper). |
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.
Please move the bulk of this comment to the main method description (instead of the parameter). Here just:
The immutable instance to be returned.
Also, please discuss ways to do something similar with a mutable object ie. regular bind() with an @provides returning spy(new Foo())
Thanks for all the comments. Unfortunately, I don't have time to work on this feature anymore this week. I will try to get to these changes when I can. |
No problem, thanks for doing this! |
What's the status on this PR? Should we only keep SpyInstanceProvider and remove the extra methods (like @PhilBeaudoin said: "My preference is to drop all these methods") ? |
shall we finish it ? |
Unfortunately guys I am much too busy in my job (DEADLINES rawr) to finish this out anytime soon. I hope the work I did is helpful to someone else should you decide to finish it, though. |
Someone want to take the lead to finish this? @przemekgalazka ? |
@durron597 @przemekgalazka Do you want to try to finish it? |
sorry but no On Fri, Apr 18, 2014 at 9:36 PM, olafleur notifications@github.com wrote:
|
Added SpyInstanceProvider functionality. Note this is not great if the instance is Mutable, but the documentation reflects this danger.