Skip to content

Comments

Code Review Bench PR #25813 - Renamed files to kebab-case - core - services - part 2#8

Open
ketkarameya wants to merge 2 commits intobase_pr_25813_20260125_4987from
corrupted_pr_25813_20260125_4987
Open

Code Review Bench PR #25813 - Renamed files to kebab-case - core - services - part 2#8
ketkarameya wants to merge 2 commits intobase_pr_25813_20260125_4987from
corrupted_pr_25813_20260125_4987

Conversation

@ketkarameya
Copy link

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#7

Continues refactoring service files to use kebab-case naming convention.

const EmailEventProcessor = require('../email-service/EmailEventProcessor');
const MailgunProvider = require('./EmailAnalyticsProviderMailgun');
const EmailAnalyticsService = require('./email-analytics-service');
const EmailEventStorage = require('../email-service/email-event-storage');

Choose a reason for hiding this comment

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

🚨 Bug: Broken requires: email-event-storage/processor files not renamed

In email-analytics-service-wrapper.js, the require paths were updated to kebab-case (../email-service/email-event-storage and ../email-service/email-event-processor), but the actual files in the email-service/ directory were not renamed — they are still EmailEventStorage.js and EmailEventProcessor.js. This will cause a runtime MODULE_NOT_FOUND error when the email analytics service initializes, breaking email analytics functionality entirely.

On case-insensitive filesystems (macOS) this might not surface during local development, but it will fail on case-sensitive filesystems (Linux, CI, production).

Was this helpful? React with 👍 / 👎

Suggested change
const EmailEventStorage = require('../email-service/email-event-storage');
const EmailEventStorage = require('../email-service/EmailEventStorage');
const EmailEventProcessor = require('../email-service/EmailEventProcessor');
  • Apply suggested fix

@@ -1,4 +1,4 @@
import {DonationPaymentEvent} from './DonationPaymentEvent';
import {DonationPaymentEvent} from './donation-payment-event';

Choose a reason for hiding this comment

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

🚨 Bug: Broken require: DonationBookshelfRepository renamed but ref not updated

The file DonationBookshelfRepository.ts was renamed to donation-bookshelf-repository.ts in this PR, but donation-service-wrapper.js (line 11) still contains require('./DonationBookshelfRepository'). Since the file no longer exists at the PascalCase path, this will cause a MODULE_NOT_FOUND error when the donation service initializes.

The fix is to update the require in donation-service-wrapper.js to use the new kebab-case filename:

const {DonationBookshelfRepository} = require('./donation-bookshelf-repository');

Was this helpful? React with 👍 / 👎

  • Apply suggested fix

@gitar-bot-staging
Copy link

Code Review 🚫 Blocked 0 resolved / 2 findings

Two critical broken require paths: email-analytics-service-wrapper.js references kebab-case email-event-storage and email-event-processor but the actual email-service/ files were never renamed; and donation-service-wrapper.js still references PascalCase DonationBookshelfRepository after the file was renamed to kebab-case.

🚨 Bug: Broken requires: email-event-storage/processor files not renamed

📄 ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js:12 📄 ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js:13

In email-analytics-service-wrapper.js, the require paths were updated to kebab-case (../email-service/email-event-storage and ../email-service/email-event-processor), but the actual files in the email-service/ directory were not renamed — they are still EmailEventStorage.js and EmailEventProcessor.js. This will cause a runtime MODULE_NOT_FOUND error when the email analytics service initializes, breaking email analytics functionality entirely.

On case-insensitive filesystems (macOS) this might not surface during local development, but it will fail on case-sensitive filesystems (Linux, CI, production).

Suggested fix
        const EmailEventStorage = require('../email-service/EmailEventStorage');
        const EmailEventProcessor = require('../email-service/EmailEventProcessor');
🚨 Bug: Broken require: DonationBookshelfRepository renamed but ref not updated

📄 ghost/core/core/server/services/donations/donation-bookshelf-repository.ts:1

The file DonationBookshelfRepository.ts was renamed to donation-bookshelf-repository.ts in this PR, but donation-service-wrapper.js (line 11) still contains require('./DonationBookshelfRepository'). Since the file no longer exists at the PascalCase path, this will cause a MODULE_NOT_FOUND error when the donation service initializes.

The fix is to update the require in donation-service-wrapper.js to use the new kebab-case filename:

const {DonationBookshelfRepository} = require('./donation-bookshelf-repository');
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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.

3 participants