Skip to content

[Reviewed] fix: add folder existence checks (Item 3)#33

Open
slendereater-sketch wants to merge 1 commit into
IN3PIRE:mainfrom
slendereater-sketch:fix/folder-existence-checks
Open

[Reviewed] fix: add folder existence checks (Item 3)#33
slendereater-sketch wants to merge 1 commit into
IN3PIRE:mainfrom
slendereater-sketch:fix/folder-existence-checks

Conversation

@slendereater-sketch
Copy link
Copy Markdown

Fixes Item 3 in Issue #21. Added existence checks for 'slashcommands' and 'events' directories to prevent startup crashes. Relates to Issue #3.

@TrivCodez
Copy link
Copy Markdown
Contributor

🔍 Pull Request Review — Issue #21 (Item 3)

This PR adds folder existence checks for slashcommands and events directories to prevent startup crashes. The approach is correct, but the branch needs rebasing before it can merge.


🔍 Changes Reviewed

  • src/bot.js: Added fs.existsSync(eventsPath) check with warning
  • src/deploy-commands.js: Added fs.existsSync(commandsPath) check and early exit with process.exit(1)
  • Total: +11 lines, -1 line across 2 files

🚨 Critical — BLOCKING MERGE

Merge conflict detected. The PR's base branch has moved ahead since this was opened, blocking automatic merge.

Fix: Rebase onto current main

git fetch upstream
git checkout fix/folder-existence-checks
git rebase upstream/main  # Resolve any conflicts manually
git push --force-with-lease

🟡 Minor Issue

Inconsistent error handlingbot.js warns and continues, but deploy-commands.js exits with code 1. Both should handle missing directories consistently. Since deploy-commands is a build-time script, exiting is correct; but the mismatch could confuse users.


✅ What's Working


⚖️ REQUEST CHANGES

Please rebase onto current main to resolve merge conflicts, then ping for merge. The implementation looks good once it's current with the base branch.

@TrivCodez TrivCodez changed the title fix: add folder existence checks (Item 3) [Reviewed] fix: add folder existence checks (Item 3) Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants