Skip to content

Fix minor code quality issues (AI findings)#109

Merged
KevinPayravi merged 1 commit into
mainfrom
fix/ai-quality-findings
May 13, 2026
Merged

Fix minor code quality issues (AI findings)#109
KevinPayravi merged 1 commit into
mainfrom
fix/ai-quality-findings

Conversation

@DominicBM
Copy link
Copy Markdown
Contributor

@DominicBM DominicBM commented May 8, 2026

Summary

Three minor fixes from GitHub AI code quality findings:

  • application.conf: Fix typo postNumberportNumber (line 44 was a duplicate key with wrong spelling — the env var POSTGRES_PORT was never actually applied)
  • ElasticSearchResponseHandler.scala: Correct "Elastic Search" → "Elasticsearch" in Scaladoc comment
  • ParamValidatorTest.scala: Remove extra space after = for consistent formatting

Test plan

  • Existing tests pass (no logic changed)
  • Verify Postgres port env var (POSTGRES_PORT) now correctly overrides the default in deployed config

🤖 Generated with Claude Code

Summary

This PR contains three minor code quality improvements:

  1. configuration.conf: Fixed a typo in the PostgreSQL configuration where postNumber (with incorrect spelling) was replaced with the correct portNumber key to properly apply the POSTGRES_PORT environment variable in deployments.

  2. ElasticSearchResponseHandler.scala: Corrected documentation comment spelling from "Elastic Search" to "Elasticsearch" (official product name).

  3. ParamValidatorTest.scala: Removed extraneous whitespace in the "subject validator" → "reject too-short param" test case for consistent formatting.

Deployment considerations

Requires ECS redeployment: The PostgreSQL port configuration fix must be deployed to production for the POSTGRES_PORT environment variable to properly override the default port value in the application configuration. This is a functional configuration change, not a no-op.

Impact

  • No logic changes — all modifications are configuration/documentation/formatting fixes
  • Test coverage: Existing tests pass without modification
  • No API changes: Public response shapes and endpoints unaffected
  • No infrastructure changes: CodePipeline, CodeBuild, ECS task definitions, and IAM policies remain unchanged

- Fix typo: postNumber → portNumber in application.conf
- Fix spelling: "Elastic Search" → "Elasticsearch" in comment
- Remove extra space in ParamValidatorTest

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8906747e-d4f0-4546-a277-1d9261a7b6e8

📥 Commits

Reviewing files that changed from the base of the PR and between a13ac2d and 75999cd.

📒 Files selected for processing (3)
  • src/main/resources/application.conf
  • src/main/scala/dpla/api/v2/search/ElasticSearchResponseHandler.scala
  • src/test/scala/dpla/api/v2/search/paramValidators/ParamValidatorTest.scala

Walkthrough

This PR contains three independent maintenance updates: the PostgreSQL port is now read from the POSTGRES_PORT environment variable instead of hardcoded, Elasticsearch terminology is corrected in a documentation comment, and test code formatting is normalized by removing extra whitespace.

Changes

Maintenance Updates

Layer / File(s) Summary
Configuration: PostgreSQL Port
src/main/resources/application.conf
portNumber changed from hardcoded "5432" to environment variable ${?POSTGRES_PORT}.
Documentation: Elasticsearch Terminology
src/main/scala/dpla/api/v2/search/ElasticSearchResponseHandler.scala
Scaladoc comment corrected from "Elastic Search" to "Elasticsearch".
Test Formatting: Whitespace Adjustment
src/test/scala/dpla/api/v2/search/paramValidators/ParamValidatorTest.scala
Test case Map construction normalized by removing extra space in argument list.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and generic, using non-descriptive phrasing like 'minor code quality issues' that doesn't clearly convey what was actually changed. Consider a more specific title such as 'Fix environment variable handling and documentation issues' to better reflect the actual changes made.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ai-quality-findings

Comment @coderabbitai help to get the list of available commands and usage tips.

@DominicBM DominicBM requested a review from KevinPayravi May 9, 2026 03:52
@KevinPayravi KevinPayravi merged commit dc4e3b3 into main May 13, 2026
5 checks passed
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