Skip to content

Add endpoint to retrieve all greetings #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kpenfound
Copy link
Owner

Add a new endpoint to the API that returns all greetings

Completed by Agent
Fixes #126

Copy link
Contributor

Test this PR by running the following command:

dagger -m github.com/kpenfound/greetings-api@pull/127/head call check

Run this branch locally:

dagger -m github.com/kpenfound/greetings-api@pull/127/head call serve up

@kpenfound
Copy link
Owner Author

/agent we inconsistently panic and os.Exit in main.go. Please standardize on panic

Repository owner deleted a comment from github-actions bot May 14, 2025
@kpenfound
Copy link
Owner Author

/review

Copy link
Contributor

Overall, the changes are well-written and accomplish the task described in the description. The new endpoint is implemented correctly, and a test case has been added to ensure that it works as expected.

Suggestions:

  • The fmt.Printf in the new handler could be removed or replaced with a logging statement.
  • Consider adding pagination to the /all endpoint if the number of greetings is expected to grow significantly.

Important changes:

  • None

Optional changes:

  • None

The pull request is ready to merge.

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.

Need an endpoint for all greetings
1 participant