Modifications to README and smbclientlogs.sh#296
Modifications to README and smbclientlogs.sh#296meetakshi253 wants to merge 5 commits intomasterfrom
Conversation
bharathsm-ms
left a comment
bharathsm-ms
left a comment
There was a problem hiding this comment.
Please make python as mandatory only for 'OnAnamoly' mode
There was a problem hiding this comment.
Pull request overview
This pull request improves the SMBDiagnostics tooling by modernizing the shell script and enhancing the README documentation. The changes focus on better coding practices, improved command checking, and clearer user-facing documentation.
Changes:
- Modernized command existence checks from
whichtocommand -vthroughout the script - Improved README formatting with proper Markdown syntax and better structure
- Enhanced error handling with return code checking for trace-cmd
- Optimized Python detection to only check when OnAnomaly mode is used
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| SMBDiagnostics/smbclientlogs.sh | Refactored utility checking logic, improved error handling for trace-cmd, optimized Python detection for OnAnomaly mode, and improved code style consistency |
| SMBDiagnostics/README | Converted to proper Markdown format with headers and code formatting, updated dependency documentation to mention package names instead of just command names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (( ($(which apt |egrep -c apt) > 0) && ($(which zgrep |egrep -c zgrep) == 0) )); | ||
| then | ||
| echo "zgrep is not installed, please install zgrep" | ||
| if (command -v apt >/dev/null 2>&1) && (! command -v zgrep >/dev/null 2>&1); then |
There was a problem hiding this comment.
Unrelated to your change: Why do we have apt as a dependency? This would not work for non-debian distros, right?
There was a problem hiding this comment.
It is a precondition for using zgrep.
| init | ||
| start_trace $@ | ||
| init "$@" | ||
| start_trace "$@" |
There was a problem hiding this comment.
Is start_trace utilizing the args passed?
There was a problem hiding this comment.
Actually, yes for VerboseLogs, but I found a bug. Let me submit another commit.
meetakshi253
force-pushed
the
msetiya/smbclientlogs
branch
from
0382f9d to
6a7632a
Compare
meetakshi253
force-pushed
the
msetiya/smbclientlogs
branch
from
6a7632a to
f7e01f7
Compare
|
@microsoft-github-policy-service agree company="Microsoft" |
No description provided.