Skip to content

Comments

Initial Support For GameLift#162

Closed
dpalvolgyi-pw wants to merge 5 commits intoclawscli:developfrom
dpalvolgyi-pw:main
Closed

Initial Support For GameLift#162
dpalvolgyi-pw wants to merge 5 commits intoclawscli:developfrom
dpalvolgyi-pw:main

Conversation

@dpalvolgyi-pw
Copy link
Contributor

@dpalvolgyi-pw dpalvolgyi-pw commented Feb 10, 2026

I'm not much of a gopher myself, just wanted to not rely on the webui for gamelift.

Any feedback is appreciated

Copy link
Contributor

@yimsk yimsk left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! The overall structure follows our patterns well — proper use of DAO/Renderer/Actions, navigation between resources, and the Supports() override for game-sessions.

A few things before we can merge:

1. Base branch → develop
Our workflow is feature → develop → main. Could you retarget this PR to develop?

2. Bug: apperrors.Wrapf with nil error (3 places)
When the API succeeds but returns empty results, err is nil at that point. Since Wrapf returns nil for nil errors, these lines return nil, nil — which will cause a nil pointer panic downstream:

  • fleets/dao.go:67
  • game-session-queues/dao.go:68
  • matchmaking-configs/dao.go:46

Fix:

// before (err is nil here, so Wrapf returns nil)
return nil, apperrors.Wrapf(err, "gamelift fleet %s not found", id)

// after
return nil, fmt.Errorf("gamelift fleet %s not found", id)

@dpalvolgyi-pw dpalvolgyi-pw changed the base branch from main to develop February 11, 2026 12:26
@dpalvolgyi-pw
Copy link
Contributor Author

closing in favor of #163

dpalvolgyi-pw added a commit to dpalvolgyi-pw/claws that referenced this pull request Feb 11, 2026
This commit adresses concerns form the comments on PR clawscli#162
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