Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Introduce public setter for Activity #29208

Merged
lmolkova merged 2 commits intodotnet:masterfrom
lmolkova:lmolkova/ActivitySetterPublic
Apr 26, 2018
Merged

Introduce public setter for Activity #29208
lmolkova merged 2 commits intodotnet:masterfrom
lmolkova:lmolkova/ActivitySetterPublic

Conversation

@lmolkova
Copy link
Copy Markdown

See #29207

@lmolkova lmolkova requested a review from vancem April 19, 2018 18:00
@vancem
Copy link
Copy Markdown

vancem commented Apr 19, 2018

I will take a look today.


private static bool ValidateSetCurrent(Activity activity)
{
bool canSet = activity == null || (activity.Id != null && !activity.isFinished);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are potential races here, but I believe the right answer is to simply tolerate them.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agree, concurrent usage and modification of Activity.Current should be extremely rare and does not look like a valid scenario

@lmolkova
Copy link
Copy Markdown
Author

@dotnet-bot test OSX x64 Debug Build

@lmolkova
Copy link
Copy Markdown
Author

@dotnet-bot test Packaging All Configurations x64 Debug Build

@lmolkova
Copy link
Copy Markdown
Author

thanks for the review, @vancem!

@dotnet dotnet deleted a comment from dotnet-bot Apr 20, 2018
@dotnet dotnet deleted a comment from dotnet-bot Apr 20, 2018
@lmolkova lmolkova changed the title [WIP] Introduce public setter for Activity Introduce public setter for Activity Apr 20, 2018
@karelz karelz added this to the 2.2.0 milestone Apr 21, 2018
@lmolkova lmolkova merged commit 7f7a789 into dotnet:master Apr 26, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…ySetterPublic

Introduce public setter for Activity.Current 

Commit migrated from dotnet/corefx@7f7a789
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants