-
Notifications
You must be signed in to change notification settings - Fork 1
feat(kc-compat): add --report flag to output system info before status #4
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
base: master
Are you sure you want to change the base?
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.
Pull Request Overview
This PR adds a new --report
flag to the kc-compat tool that outputs detailed system information before displaying the compatibility status. This enhances the tool's diagnostic capabilities for support scenarios.
- Refactors kernel hash generation into a testable function that accepts data directly
- Adds
--report
flag functionality to display system information including kernel hash, distribution, and kernel version - Updates tests to reflect the new function signature and adds comprehensive test coverage for report mode
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
kc-compat.py | Implements report mode functionality and refactors kernel hash generation for better testability |
test_kc_compat.py | Updates existing tests for new function signatures and adds test coverage for report mode |
README.md | Documents the new --report flag with usage examples and output format |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
=== KernelCare Compatibility Report === | ||
Kernel Hash: abcdef1234567890abcdef1234567890abcdef12 | ||
Distribution: centos | ||
Version: 7 |
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.
The documentation shows 'Version: 7' in the example output, but the actual code on line 136 in kc-compat.py always prints 'Version: Not available'. This inconsistency could confuse users about what to expect from the --report output.
Version: 7 | |
Version: Not available |
Copilot uses AI. Check for mistakes.
Distribution: centos | ||
Version: 7 | ||
Kernel: Linux version 5.4.0-74-generic (buildd@lcy01-amd64-023) (gcc version 9.3.0) | ||
Environment: Physical/Virtual Machine |
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.
The documentation shows 'Environment: Physical/Virtual Machine' in the example output, but this line is not implemented in the actual code. The code only outputs kernel hash, distribution, version (hardcoded as 'Not available'), and kernel information.
Environment: Physical/Virtual Machine |
Copilot uses AI. Check for mistakes.
else exit with 0 if COMPATIBLE, 1 or more otherwise | ||
""" | ||
silent = len(sys.argv) > 1 and (sys.argv[1] == '--silent' or sys.argv[1] == '-q') | ||
report = len(sys.argv) > 1 and sys.argv[1] == '--report' |
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.
The flag parsing logic doesn't handle multiple arguments correctly. If both --silent and --report are provided, or if --report is provided as a second argument, the current logic will fail to detect it properly. Consider using argparse or at least checking all argv elements instead of just argv[1].
silent = len(sys.argv) > 1 and (sys.argv[1] == '--silent' or sys.argv[1] == '-q') | |
report = len(sys.argv) > 1 and sys.argv[1] == '--report' | |
silent = ('--silent' in sys.argv[1:]) or ('-q' in sys.argv[1:]) | |
report = '--report' in sys.argv[1:] |
Copilot uses AI. Check for mistakes.
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.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
silent = len(sys.argv) > 1 and (sys.argv[1] == '--silent' or sys.argv[1] == '-q') | ||
report = len(sys.argv) > 1 and sys.argv[1] == '--report' |
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.
The current argument parsing logic only checks the first argument, making it impossible to use both --silent and --report flags together or handle multiple arguments properly. Consider using argparse for proper command-line argument handling.
Copilot uses AI. Check for mistakes.
No description provided.