-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
Add support for resource policies #276
Conversation
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.
Copilot reviewed 5 out of 9 changed files in this pull request and generated no comments.
Files not reviewed (4)
- CliWrap/Utils/ProcessEx.cs: Evaluated as low risk
- CliWrap/ICommandConfiguration.cs: Evaluated as low risk
- CliWrap/Command.cs: Evaluated as low risk
- Readme.md: Evaluated as low risk
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #276 +/- ##
==========================================
+ Coverage 95.05% 95.37% +0.31%
==========================================
Files 46 48 +2
Lines 1113 1188 +75
Branches 85 86 +1
==========================================
+ Hits 1058 1133 +75
Misses 35 35
Partials 20 20 ☔ View full report in Codecov by Sentry. |
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.
Copilot reviewed 5 out of 10 changed files in this pull request and generated no comments.
Files not reviewed (5)
- CliWrap.Tests/CredentialsSpecs.cs: Evaluated as low risk
- CliWrap/Command.Execution.cs: Evaluated as low risk
- CliWrap/ResourcePolicy.cs: Evaluated as low risk
- CliWrap/Utils/ProcessEx.cs: Evaluated as low risk
- CliWrap/ICommandConfiguration.cs: Evaluated as low risk
Comments suppressed due to low confidence (1)
CliWrap.Tests/ResourcePolicySpecs.cs:19
- [nitpick] The error message should be clearer. Suggestion: 'Setting process priority requires elevated permissions on non-Windows platforms, which cannot be guaranteed in a CI environment. Therefore, this test is only run on Windows.'
Starting a process with a custom priority is only supported on Windows.
This PR adds a new configuration method called
WithResourcePolicy(...)
, along with a builder that helps the user configure process's priority, core affinity, and working set.Closes #266