Add farewells

[kpenfound]

The greetings-api is so cool and greetings are just great.
What if the API could also return farewells instead of greetings?

I've implemented the API changes needed to add farewells

[agent-kal]

Suggested change

	}
	func FormatResponse(greeting *Greeting) string {
	return fmt.Sprintf("{\"greeting\":\"%s\"}", greeting.Greeting)
	}

[kpenfound]

/review

[agent-kal]

Pull Request Review

General Opinion

This pull request successfully implements farewell functionality by mirroring the existing greeting system. The implementation follows the same patterns and structures as the existing code, which is good for consistency. The code accomplishes the stated task of adding farewell endpoints to the API.

Required Changes to Make PR Acceptable

1. Fix broken test: The test TestFormatResponse references the old function name FormatResponse which has been renamed to FormatGreetingResponse. This is causing the build to fail.

2. Add missing tests for new functionality: The PR introduces significant new functionality (SelectFarewell function, FormatFarewellResponse function, and new endpoints) but provides no tests for these features. Based on the existing test patterns, the following tests should be added:

   • TestSelectFarewell (equivalent to TestSelectGreeting)
   • TestFormatFarewellResponse (equivalent to the corrected TestFormatGreetingResponse)
   • Integration tests for the new /farewell and /farewell/{language} endpoints

Suggestions Within Scope

1. Consistent error handling: The existing /{language} endpoint was missing a return statement after http.Error(), which the PR correctly adds. This improvement should be acknowledged.

2. Route ordering: The PR correctly places farewell endpoints before the generic /{language} endpoint to avoid routing conflicts. This shows good understanding of the routing system.

Optional Improvements

1. JSON escaping: Both FormatGreetingResponse and FormatFarewellResponse manually construct JSON strings without proper escaping. While this works for the current content, using json.Marshal() would be more robust for handling special characters.

2. DRY principle: The farewell functionality is almost identical to greeting functionality. Consider whether this duplication could be reduced with generic functions, though this might be outside the scope of this PR.

3. Documentation: The PR doesn't update any documentation (README, API docs) to reflect the new farewell endpoints.

Code Quality Assessment

• ✅ Follows existing code patterns and structure
• ✅ Proper error handling and HTTP status codes
• ✅ Correct route ordering to avoid conflicts
• ✅ Consistent logging and response formatting
• ✅ No obviously malicious code
• ❌ Missing tests for new functionality
• ❌ Breaks existing tests

Summary

This pull request needs changes before it can be merged. The core functionality is well-implemented and follows good practices, but the broken test and missing test coverage for new features make it unacceptable in its current state. Once the test issues are resolved, this would be a solid addition to the codebase.

The implementation correctly adds farewell functionality as requested, with proper API endpoints (/farewell and /farewell/{language}) that mirror the existing greeting system. The code quality is good and consistent with the existing codebase.