-
Notifications
You must be signed in to change notification settings - Fork 23
refactor!: Major code cleanup for clearer state #27
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
Merged
Merged
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
0a38386
Quick pass
JR-Morgan 89746b4
no locks
JR-Morgan 0f9618e
github actions
JR-Morgan f15d23b
publish via github actions
JR-Morgan 46aa097
pack
JR-Morgan 53cc4b7
Parent non-optional
JR-Morgan 567b2b1
Generics are fun
JR-Morgan 4282bac
second pass
JR-Morgan 99aa4b2
Group workers, CTS, and Task in a class for simpler nullability
JR-Morgan 01ae032
pollysharp
JR-Morgan c162617
reverted cancellation changes
JR-Morgan a71ffaf
reset state
JR-Morgan 6ac757d
Update samples
JR-Morgan 5fab08a
changes
JR-Morgan 9950e51
Merge branch 'jrm/confirmed-working' into jrm/sourcelink
JR-Morgan 32e1ba6
still working
JR-Morgan f28f1d8
merge
JR-Morgan 2575783
the fix
JR-Morgan 26dea0e
merge
JR-Morgan 6674e46
private workers
JR-Morgan 9f02d43
comments
JR-Morgan bf3026c
comments 2
JR-Morgan a0e8645
comments 3
JR-Morgan db94b91
Merge branch 'dev' into jrm/sourcelink
JR-Morgan 5800e75
public worker count
JR-Morgan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Task.Factory.StartNew()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No try/catch or other error handling for this task? Unobserved task exceptions 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's important that the tasks don't start within this SolveInstance.
They get started all together in PostSolveInstance
That way, it it guaranteed that a task never calls
Donewhile we are still spinning up workers. That would break the components state tracking.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps there's alternatives with storing
Func<Task>, but for this PR I wanted to keep minimal changes