Replaces find with ls #203
Open
BrianAtZetica wants to merge 2 commits into
Open
Conversation
…directory when checkgin for preexisting output files. Resolves Deep Find triggers OneDrive download Fixes mona-actions#202
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ERROR_CODE=$? | ||
| if [ "${ERROR_CODE}" -eq 0 ]; then | ||
| JSON_FILE_NAME="${EXISTING_JSON_CMD:2}" | ||
| EXISTING_JSON_CMD=$(ls "$ORG_NAME-all_repos-*.json" 2>/dev/null | head -1) |
| # Load the error code # | ||
| ####################### | ||
| ERROR_CODE=$? | ||
| EXISTING_FILE_CMD=$(ls "$ORG_NAME-all_repos-"* 2>/dev/null | head -1) |
| # Load the error code # | ||
| ####################### | ||
| ERROR_CODE=$? | ||
| EXISTING_FILE_CMD=$(ls "$ORG_NAME-repo-conflicts-"* 2>/dev/null | head -1) |
| # Need to see if there is a file that already exists # | ||
| ###################################################### | ||
| EXISTING_FILE_CMD=$(find . -name "$ORG_NAME-team-conflicts-*" |grep . 2>&1) | ||
| EXISTING_FILE_CMD=$(ls "$ORG_NAME-team-conflicts-"* 2>/dev/null | head -1) |
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces recursive find calls used to detect pre-existing output files with non-recursive ls invocations, addressing issue #202 where find traversed the entire user directory tree (causing OneDrive to download all files and producing permission-denied errors on system folders). Also adds Debug log lines for the found/not-found cases.
Changes:
- Replaces four
find . -name ...invocations withlspatterns scoped to the current directory. - Switches the success check from
$?(aftergrep .) to a-ntest on the captured output. - Adds Debug messages for both the "found existing" and "creating new" branches.
Show a summary per file
| File | Description |
|---|---|
| gh-repo-stats | Swaps recursive find with ls globs and adds Debug output in GenerateFiles for the JSON, CSV, repo-conflicts, and team-conflicts file lookups. |
Notable findings:
- Line 388 has the glob fully inside double quotes (
ls "$ORG_NAME-all_repos-*.json"), which prevents shell expansion — this lookup will never find an existing JSON file. The other three sites correctly close the quote before*. - Behavior change: the new
lsapproach only inspects the current working directory, whereasfind .previously recursed. Depending on how the script is invoked, this may stop matching legitimate pre-existing outputs. - Minor indentation/whitespace inconsistencies around lines 462–464 and trailing spaces after some Debug lines.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 4
| # Load the error code # | ||
| ####################### | ||
| ERROR_CODE=$? | ||
| EXISTING_FILE_CMD=$(ls "$ORG_NAME-all_repos-"* 2>/dev/null | head -1) |
Contributor
Author
There was a problem hiding this comment.
I'm unable to resolve this concern. Switchign to ls and looking non-recursively solved my problem of an unexpected OneDrive dowload trigger. If it isnt appropriate for all users, you'll have to reject the PR. Thanks
Comment on lines
+462
to
+464
| ################################ | ||
| # Check ls result is not empty # | ||
| ################################ |
| JSON_FILE_NAME="${EXISTING_JSON_CMD}" | ||
| Debug "Found existing JSON file, using: $JSON_FILE_NAME" | ||
| else | ||
| Debug "No existing JSON file found, creating: $JSON_FILE_NAME" |
corrected glob pattern, which incorrectly enclosed the wildcard in quotes Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Brian Barrett <119825052+BrianAtZetica@users.noreply.github.com>
Contributor
|
@BrianAtZetica Instead of switching to |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This ensures it doesnt recursively search a users entire directory when checking for preexisting output files. It now only looks in the current directory.
Resolves issue where access errors were reported after find tried to search system folders and where OneDrive woudl start downlaoding all users files due to being touched by find
Fixes #202