-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat: add exec command support for xlinectl lock #871
Conversation
@bhavik-goplani Convert your pr to draft since CI failed |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #871 +/- ##
==========================================
+ Coverage 75.55% 75.99% +0.43%
==========================================
Files 180 189 +9
Lines 26938 28405 +1467
Branches 26938 28405 +1467
==========================================
+ Hits 20353 21585 +1232
- Misses 5366 5486 +120
- Partials 1219 1334 +115 ☔ View full report in Codecov by Sentry. |
Hi, Bhavik! I really appreciate your passion and resilience. Please add some unit test, and squash your commits into one. |
294f0b0
to
f100b95
Compare
@CrystalAnalyst Thanks for your feedback! I have made the final changes and added unit tests. |
Signed-off-by: bhavik-goplani <56516858+bhavik-goplani@users.noreply.github.com>
@CrystalAnalyst Do you think I need to add more tests to hit the 75.5 % diff target? I think adding more test cases is redundant here. |
Please briefly answer these questions:
what problem are you trying to solve? (or if there's no problem, what's the motivation for this change?)
To solve issue: [Feature]: Add exec command args for xlinectl lock command #823
what changes does this pull request make?
Change the
xlinectl lock
commandare there any non-obvious implications of these changes? (does it break compatibility with previous versions, etc)