Skip to content

Use static readonly for default ExecutionOptions #1703

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 15, 2022
Merged

Use static readonly for default ExecutionOptions #1703

merged 1 commit into from
Feb 15, 2022

Conversation

andyleejordan
Copy link
Member

But at the SynchronousTask level. Now we get the best of both worlds:
the records have the correct default values on initialization, and we
only heap allocate a single static instance of ExecutionOptions for
the default case.

@andyleejordan
Use static readonly for default ExecutionOptions
8a9f1eb 
But at the `SynchronousTask` level. Now we get the best of both worlds:
the records have the correct default values on initialization, and we
only heap allocate a single static instance of `ExecutionOptions` for
the default case.
@andyleejordan andyleejordan added the Ignore Exclude from the changelog. label Feb 11, 2022
@andyleejordan
Copy link
Member Author

@PaulHigin Originally I was fixing up DebugService.cs to set AddToHistory = false, thinking that was the issue; however, that was the default and instead I discovered that those InvocationSettings aren't passed at all in ExecuteInDebugger. Anyway, while working on that I cleaned this up a little bit.

@andyleejordan andyleejordan added Area-Debugging Issue-Enhancement A feature request (enhancement). and removed Ignore Exclude from the changelog. labels Feb 15, 2022
@andyleejordan andyleejordan enabled auto-merge (squash) February 15, 2022 22:36
@andyleejordan
Copy link
Member Author

Reviewed this over the phone with Paul, merging!

@andyleejordan andyleejordan enabled auto-merge (squash) February 15, 2022 23:08
@andyleejordan
Copy link
Member Author

Err, GitHub still has that bug we're I can't squash and merge if I use override as admin privileges.

@andyleejordan andyleejordan merged commit 6754359 into master Feb 15, 2022
@andyleejordan andyleejordan deleted the andschwa/static-default branch February 15, 2022 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SeeminglyScience SeeminglyScience Awaiting requested review from SeeminglyScience

1 more reviewer

@PaulHigin PaulHigin PaulHigin approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Area-Debugging Issue-Enhancement A feature request (enhancement).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andyleejordan @PaulHigin