Skip to content

Add code review document#34

Open
ginomessmer wants to merge 4 commits into
mainfrom
claude/code-review-odJfI
Open

Add code review document#34
ginomessmer wants to merge 4 commits into
mainfrom
claude/code-review-odJfI

Conversation

@ginomessmer
Copy link
Copy Markdown
Owner

Comprehensive review covering the Discord bot (C#/.NET 8),
Azure Bicep infrastructure, Dockerfile, and GitHub Actions CI/CD.

https://claude.ai/code/session_01KQLiBWpQThmXMQRG7jfZ1t

Comprehensive review covering the Discord bot (C#/.NET 8),
Azure Bicep infrastructure, Dockerfile, and GitHub Actions CI/CD.

https://claude.ai/code/session_01KQLiBWpQThmXMQRG7jfZ1t
- Remove eager .Get() call on ContainerGroupResource singleton so the
  bot no longer crashes at startup if Azure is unreachable, and the
  resource handle no longer captures stale state
- Fetch fresh container data in /status command via GetAsync()
- Replace busy-wait polling loop with TaskCompletionSource for
  interaction responses (eliminates wasted CPU and up to 100ms latency)
- Properly dispose StreamReader with leaveOpen: true
- Make Docker Hub imageRegistryCredentials conditional so empty
  credentials don't break pulls of the public itzg/minecraft-server image

https://claude.ai/code/session_01KQLiBWpQThmXMQRG7jfZ1t
- Use concat() for conditional Bicep array items instead of emitting
  empty objects when Bedrock is disabled (#6)
- Only set RESOURCE_PACK env var when resourcePackUrl is non-empty (#7)
- Check stoppingToken before each async operation in
  BotBackgroundService so shutdown is respected during startup (#8)
- Pin azure/login to same commit hash across all workflows (#9)
- Replace deprecated ::set-output syntax with $GITHUB_OUTPUT in
  destroy.yml (#10)
- Fix broken key::value syntax to key=value in server.yml (#11)
- Remove duplicate InvariantGlobalization property in csproj (#12)
- Add RequireUserPermission(ManageGuild) to /start and /stop
  commands (#13)
- Document intentional allowBlobPublicAccess on storage accounts (#14)

https://claude.ai/code/session_01KQLiBWpQThmXMQRG7jfZ1t
- Replace curl with wget in Dockerfile HEALTHCHECK since the base
  aspnet image doesn't include curl (#15)
- Fix auto-shutdown param description to say 3:00 AM instead of
  midnight to match the actual default schedule (#16)
- Change webMapFqdn output to nullable string? for consistency with
  discordInteractionEndpoint (#17)
- Select correct Geyser download variant (forge vs spigot) based on
  serverType (#18)
- .dockerignore already exists, no change needed (#19)
- Move > whatif redirect from shared AZ_INLINE_SCRIPT_PARAMS env var
  into the what-if step only, so deploy step output isn't swallowed (#20)
- Add explicit permissions (contents: read, packages: write) to
  release.yml (#22)

https://claude.ai/code/session_01KQLiBWpQThmXMQRG7jfZ1t
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants