-
Notifications
You must be signed in to change notification settings - Fork 831
Make Resharper green again #1042
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
Conversation
…erity from Warning to Suggestion
Great! Will have a look in detail ASAP.
This is a false positive, can be ignored.
I am not 100% confident this can never occur, could ignore with a comment to that effect.
This is by design and can be ignored. |
Thanks, I've updated with your comments :) Out of curiosity, I looked up public override Object InitializeLifetimeService()
{
BCLDebug.Trace("REMOTE", "ObjectHandle.InitializeLifetimeService");
//
// If the wrapped object has implemented InitializeLifetimeService to return null,
// we don't want to go to the base class (which will result in a lease being
// requested from the MarshalByRefObject, which starts up the LeaseManager,
// which starts up the ThreadPool, adding three threads to the process.
// We check if the wrapped object is a MarshalByRef object, and call InitializeLifetimeServices on it
// and if it returns null, we return null. Otherwise we fall back to the old behavior.
//
MarshalByRefObject mbr = WrappedObject as MarshalByRefObject;
if (mbr != null) {
Object o = mbr.InitializeLifetimeService();
if (o == null)
return null;
}
ILease lease = (ILease)base.InitializeLifetimeService();
return lease;
} .... so it may indeed return null. Maybe serilog/src/Serilog/Context/LogContext.cs Lines 254 to 259 in 8528ae3
could be changed to become: public override object InitializeLifetimeService()
{
var lease = base.InitializeLifetimeService() as ILease;
lease?.Register(LifeTimeSponsor);
return lease;
} |
That would be a fair change. I don't think we ever create a handle to a Will merge as-is so that we don't drift into merge-conflict territory in the meantime :-) |
What issue does this PR address?
Get rid of errors and warnings reported by ReSharper Solution-wide analysis
Does this PR introduce a breaking change?
Nope
Please check if the PR fulfills these requirements
Other information:

Before :
After :

There are still some warnings; but I was not sure how to fix them :