feat :: (#2) setup 파일 생성 #3
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughTwo new files are added to the repository. README.md receives extensive Korean documentation describing the setup.sh script's purpose, usage, and post-execution steps. A new setup.sh script is introduced for automating Android project template configuration, including package name restructuring, source directory reorganization, gradle configuration updates, resource file modifications, and project directory renaming. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 50: Remove the stray Korean laughter character "ㅋ" from the sentence that
currently reads "Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다.ㅋ" in the README;
update it to "Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다." and verify surrounding
punctuation/spacing is unchanged.
In `@setup.sh`:
- Around line 184-187: The current copy step uses cp -R "$src"/* "$dst"/ which
skips dotfiles; update the block that checks the source directory (uses
variables src and dst and the ls -A test) to temporarily enable bash dotglob
(shopt -s dotglob) before running cp -R "$src"/* "$dst"/ and then disable it
(shopt -u dotglob) so hidden files like .gitkeep are included; keep the existing
presence check to avoid copying when empty.
🧹 Nitpick comments (3)
setup.sh (3)
17-61: Consider separating variable declaration and assignment.Shellcheck SC2155 warns that combining
localwith command substitution masks the command's return value. While this script handles detection gracefully (checking for empty values later), separating declaration and assignment is a defensive practice.♻️ Example fix pattern
- local app_kt_file=$(find app/src/main/java -name "*.kt" -type f 2>/dev/null | head -1) + local app_kt_file + app_kt_file=$(find app/src/main/java -name "*.kt" -type f 2>/dev/null | head -1)Apply the same pattern to other
local var=$(...)assignments in this function and elsewhere in the script.
124-136: Sed patterns are not escaped for regex metacharacters.The
$oldvariable is used directly in the sed pattern, but package names contain.which is a regex metacharacter matching any character. For example,com.example.appwould also technically matchcom_example_app.In practice this is unlikely to cause issues for this use case, but escaping the pattern would be more robust.
♻️ Proposed fix with escaping
replace_in_file() { local file="$1" local old="$2" local new="$3" + # Escape regex metacharacters in old pattern + local escaped_old + escaped_old=$(printf '%s\n' "$old" | sed 's/[.[\*^$()+?{|]/\\&/g') if [[ -f "$file" ]]; then if [[ "$OSTYPE" == "darwin"* ]]; then - sed -i '' "s|$old|$new|g" "$file" + sed -i '' "s|$escaped_old|$new|g" "$file" else - sed -i "s|$old|$new|g" "$file" + sed -i "s|$escaped_old|$new|g" "$file" fi fi }
406-416: Consider adding explicit error handling for directory rename.If the
mvcommand fails (e.g., due to permissions or the target already existing), theset -ewill abort the script. Adding explicit error handling would provide a better user experience with a clear error message.♻️ Proposed improvement
if [[ "$CURRENT_DIR" != "$NEW_PROJECT_DIR" ]]; then + if [[ -e "$NEW_PROJECT_DIR" ]]; then + log_error "Target directory already exists: $NEW_PROJECT_DIR" + log_warn "Setup completed but project directory was not renamed." + exit 1 + fi cd "$PARENT_DIR" - mv "$CURRENT_DIR" "$NEW_PROJECT_DIR" + if ! mv "$CURRENT_DIR" "$NEW_PROJECT_DIR"; then + log_error "Failed to rename project directory" + cd "$CURRENT_DIR" + exit 1 + fi log_success "Renamed project directory: $(basename "$CURRENT_DIR") -> $NEW_PROJECT_DIR_NAME" cd "$NEW_PROJECT_DIR" fi
README.md
Outdated
| ```bash | ||
| cd /path/to/your-new-app-name | ||
| ``` | ||
| 2. **Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다**.ㅋ |
There was a problem hiding this comment.
Remove errant character "ㅋ" from documentation.
Line 50 contains "엽니다.ㅋ" where "ㅋ" appears to be an accidental keystroke (Korean slang for laughter). This should be removed from the documentation.
✏️ Proposed fix
-2. **Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다**.ㅋ
+2. **Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다**.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 2. **Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다**.ㅋ | |
| 2. **Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다**. |
🤖 Prompt for AI Agents
In `@README.md` at line 50, Remove the stray Korean laughter character "ㅋ" from
the sentence that currently reads "Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다.ㅋ"
in the README; update it to "Android Studio (또는 선호하는 IDE)에서 프로젝트를 엽니다." and
verify surrounding punctuation/spacing is unchanged.
| # Copy all contents | ||
| if [[ -n "$(ls -A "$src" 2>/dev/null)" ]]; then | ||
| cp -R "$src"/* "$dst"/ | ||
| fi |
There was a problem hiding this comment.
Hidden files (dotfiles) are not copied.
The glob "$src"/* does not match hidden files like .gitkeep. This could result in data loss if the source directories contain dotfiles.
🐛 Proposed fix to copy all files including hidden ones
# Copy all contents
if [[ -n "$(ls -A "$src" 2>/dev/null)" ]]; then
- cp -R "$src"/* "$dst"/
+ # Copy all files including hidden files
+ cp -R "$src"/. "$dst"/
fiAlternatively, enable dotglob temporarily:
shopt -s dotglob
cp -R "$src"/* "$dst"/
shopt -u dotglob📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Copy all contents | |
| if [[ -n "$(ls -A "$src" 2>/dev/null)" ]]; then | |
| cp -R "$src"/* "$dst"/ | |
| fi | |
| # Copy all contents | |
| if [[ -n "$(ls -A "$src" 2>/dev/null)" ]]; then | |
| # Copy all files including hidden files | |
| cp -R "$src"/. "$dst"/ | |
| fi |
🤖 Prompt for AI Agents
In `@setup.sh` around lines 184 - 187, The current copy step uses cp -R "$src"/*
"$dst"/ which skips dotfiles; update the block that checks the source directory
(uses variables src and dst and the ls -A test) to temporarily enable bash
dotglob (shopt -s dotglob) before running cp -R "$src"/* "$dst"/ and then
disable it (shopt -u dotglob) so hidden files like .gitkeep are included; keep
the existing presence check to avoid copying when empty.
Summary by CodeRabbit
Release Notes
New Features
Documentation