Skip to content
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

Adjust nested ambient transaction exception message #20089

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

AndriySvyryd
Copy link
Member

No description provided.

Copy link
Contributor

@dshuvaev dshuvaev left a comment

Choose a reason for hiding this comment

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

Cool. I have a question about injecting System.Transactions.Transaction.Current to RelationalConnection. Would it better to inject it properly using some kind of factory, or just put a protected virtual method here? I personally against using virtual method for sake of unit testing, on the other hand factory that has just one implementation for returning this static property is probably too much as well. What do you think?

@AndriySvyryd
Copy link
Member Author

@dshuvaev I'm not following you. Why would you need to inject Transaction.Current if it's static and you could just set it?

@dshuvaev
Copy link
Contributor

dshuvaev commented Mar 2, 2020

@dshuvaev I'm not following you. Why would you need to inject Transaction.Current if it's static and you could just set it?

It is a good point. On the other hand having such a dependency to be injected indirectly does not seem right to me. Nonetheless, if you think it does not make sense to do any changes there it is fine.

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