Chang this warning to DEBUG level output

[haacked]

This just adds noise to our log files. Thoughts on making it a DEBUG warning?

[shiftkey]

👍

[anaisbetts]

So, this warning isn't just for funsies, it indicates something that can be a real bug and troll the fuck out of you if you didn't realize what's happening - it means that if you do something like:

this.WhenAnyValue(x => x.Foo.Bar.Baz)
    .Subscribe(Console.WriteLine);

// Later, in another part of town...

// We changed Foo, but since it's a POCO object, WhenAnyValue won't work correctly because
// it didn't know to switch to the new Foo.
this.Foo = new BarType();      

// Later, when you're about to ship
Console.WriteLine("WHY THE FUCK DOESNT MY CONSOLE WRITELINE FIRE OMG IM ABOUT TO BE FIRED THIS IS TERRIBLE");

[anaisbetts]

That could be a real Changmergency!

[haacked]

Can we make it a #DEBUG only warning? How do I turn off this warning in my app?

[anaisbetts]

@haacked Make a static property on POCOObservableForProperty called DisablePOCOWarningBecauseIHerebyPromiseThatIUnderstandTheConsequencesOfThisDecision that disables the warning

[jlaanstra]

How about using the [Conditional("DEBUG")] attribute? This way you see the warnings when debugging an app but you won't see it in release mode. The warning has to get its own method though.

[shiftkey]

The big issue for me at least is that we can't resolve these warnings - because they're UserControls or types we don't own.

If we could treat these like FxCop rules and handle/mute them at compile time, it'd be ✨✨✨

[anaisbetts]

This way you see the warnings when debugging an app but you won't see it in release mode.

Are you sure? I'm pretty sure that this will strip the method completely in the Release build of ReactiveUI.dll, no? (i.e, it's not the app's Debug/Release, it's this library's)

The problem is, nobody uses the Debug build of ReactiveUI.dll - otherwise there's a ton of stuff I'd downgrade to debug-only. Because this isn't tracing a bug in RxUI, it's helping users to trace a bug in their app

[jlaanstra]

Are you sure? I'm pretty sure that this will strip the method completely in the Release build of ReactiveUI.dll, no? (i.e, it's not the app's Debug/Release, it's this library's)

No :). Methods with the Conditional attribute will not be removed from the release build. They will be there, but when the specified flag is not defined it will not be compiled to MSIL (http://msdn.microsoft.com/en-us/library/vstudio/system.diagnostics.conditionalattribute). This is actually how the Debug class works in .NET.

[haacked]

@jlaanstra that documentation directly contradicts what you just said. 😄

Indicates to compilers that a method call or attribute should be ignored unless a specified conditional compilation symbol is defined.

AFAIK, ReactiveUI release builds do not specify the DEBUG conditional compilation symbol thus the method would not be compiled into the release build.

[haacked]

@jlaanstra sorry, I should have read further. My reading comprehension skills are not so good.

[haacked]

And I just verified @jlaanstra's claim with Reflector. 😄 Learn something new every day.

In retrospect, it makes total sense. Why would csc.exe change its behavior based on an attribute when we already have other #conditionals.

[jlaanstra]

And I just verified @jlaanstra's claim with Reflector. 😄 Learn something new every day.

Spread the word! :) Not enough people know about this feature, while it can be very useful.

[anaisbetts]

@jlaanstra In that case I am +:100: to marking that with [Conditional]

[haacked]

@jlaanstra will do! I feel a blog post coming up.

[jlaanstra]

@jlaanstra In that case I am +:100: to marking that with [Conditional]

...otherwise there's a ton of stuff I'd downgrade to debug-only.

So what are other places where we would like to have this? There are a couple more of these "You might be breaking your app" warnings where we should add this I think.

[anaisbetts]

We can do this whenever, punted (though you can still make it in RxUI 6.x of course)

[ghuntley]

Let's schedule this in for RxUI v7? Are there any other areas you want to also slap with [Conditional("DEBUG")]?

[kentcb]

I think there's still some misunderstanding (possibly mine) of the conditional compilation feature. How would we use it in this scenario?

We can't mark GetNotificationForProperty as conditional because it has a return value (if you think about it, this makes sense because how would it be removed if the call site used the return value in a subsequent calculation?).

And if we were to separate the logging call into a separate method and mark that with Conditional("DEBUG"), it would be ReactiveUI calling that method, not the client code. Therefore, its inclusion would be contingent upon the compilation settings of ReactiveUI, not the client library.

Conditional compilation only works for parameterless methods that the client code calls directly, unless I'm missing something.

[ghuntley]