Skip to content

[Reviewed] feat: support guild scoped command deploys#37

Open
ixhxpns wants to merge 1 commit into
IN3PIRE:mainfrom
ixhxpns:codex/item-8-guild-scope
Open

[Reviewed] feat: support guild scoped command deploys#37
ixhxpns wants to merge 1 commit into
IN3PIRE:mainfrom
ixhxpns:codex/item-8-guild-scope

Conversation

@ixhxpns
Copy link
Copy Markdown
Contributor

@ixhxpns ixhxpns commented Apr 29, 2026

Summary\n- support GUILD_ID in src/deploy-commands.js\n- deploy to Routes.applicationGuildCommands when GUILD_ID is set\n- keep existing global deploy behavior when GUILD_ID is not set\n\n## Bounty\nResolves Item #8 from #21.\n\n## Verification\n- node --check src/deploy-commands.js\n\nI did not run a live Discord deploy because local Discord credentials are not configured.\n

Copy link
Copy Markdown
Contributor

@TrivCodez TrivCodez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes due to PR formatting.

The PR description is too minimal for a production-ready codebase. Please structure it following the project's contribution standards:

What should be included:

  1. What: Clear explanation of the feature - guild-scoped command deployment
  2. Why: Explain the benefit of supporting GUILD_ID - faster command testing/updates vs global deployment latency
  3. Changes: Detail the specific files and logic changes
  4. Testing: How was this verified? The node --check line is helpful, but explain what scenarios were covered
  5. Backwards compatibility: Confirm global deployment still works when GUILD_ID is unset

Missing details:

  • No explanation of how Routes.applicationGuildCommands differs from global routes
  • No documentation of environment variable usage
  • No mention of discord.js version compatibility implications
  • No testing evidence beyond syntax checking

Please update the PR description with these details before this can be reviewed for merge. The implementation may be fine, but the communication needs to match the standards expected for a production bot handler.

Once updated, this PR can move forward for technical review.

@TrivCodez
Copy link
Copy Markdown
Contributor

Hey @ixhxpns - thanks for the contribution!

I've left a review requesting changes primarily around PR description formatting. The implementation itself looks solid (guild-scoped deploy is a great feature addition), but the PR description needs more detail to meet the project's standards for a production-ready codebase.

Could you update the PR description with:

  • Clear explanation of what guild-scoped deployment does and why it's beneficial
  • Details about the implementation approach
  • Environment variable usage documentation
  • Confirmation of backwards compatibility
  • Any testing beyond the syntax check

Once the description is updated, we can move forward with the technical review and get this merged. The code changes themselves look good at first glance!

@TrivCodez TrivCodez changed the title feat: support guild scoped command deploys [Reviewed] feat: support guild scoped command deploys Apr 30, 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