Renamed files to kebab-case - core - services - part 2#9
Conversation
Code Review by Qodo
1. Missing semicolons in members-api.js
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+1
to
+10
| const {Router} = require('express') | ||
| const body = require('body-parser') | ||
| const errors = require('@tryghost/errors') | ||
| const logging = require('@tryghost/logging') | ||
|
|
||
| const PaymentsService = require('./services/payments-service') | ||
| const TokenService = require('./services/token-service') | ||
| const GeolocationService = require('./services/geolocation-service') | ||
| const MemberBREADService = require('./services/member-bread-service') | ||
| const MemberRepository = require('./repositories/member-repository') |
There was a problem hiding this comment.
1. Missing semicolons in members-api.js 📘 Rule violation ✓ Correctness
• Several newly added require(...) statements in members-api.js omit semicolons, relying on ASI. • This violates the repo style requirement that statements end with semicolons and introduces inconsistency in the same file where later require lines do use semicolons. • Inconsistent semicolon usage can lead to subtle parsing issues and reduces codebase readability/consistency.
Agent prompt
## Issue description
Several newly added `require(...)` statements in `members-api.js` are missing trailing semicolons, violating the repo JS style rules.
## Issue Context
This file mixes semicolon-less and semicolon-terminated statements, which is inconsistent and against the compliance checklist requirement.
## Fix Focus Areas
- ghost/core/core/server/services/members/members-api/members-api.js[1-10]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+12
to
+13
| const EmailEventStorage = require('../email-service/email-event-storage'); | ||
| const EmailEventProcessor = require('../email-service/email-event-processor'); |
There was a problem hiding this comment.
2. Email analytics require mismatch 🐞 Bug ✓ Correctness
• EmailAnalyticsServiceWrapper now requires ../email-service/email-event-storage and ../email-service/email-event-processor, but the repo contains EmailEventStorage.js and EmailEventProcessor.js (PascalCase). • On case-sensitive filesystems, Node will not resolve these mismatched paths, causing Email Analytics init to fail with MODULE_NOT_FOUND. • Impact: email analytics worker/service initialization can crash at startup, preventing analytics ingestion/processing.
Agent prompt
## Issue description
`EmailAnalyticsServiceWrapper` requires `../email-service/email-event-storage` and `../email-service/email-event-processor`, but the email-service implementations in this repo are `EmailEventStorage.js` and `EmailEventProcessor.js`. On case-sensitive filesystems this will throw `MODULE_NOT_FOUND` when initializing Email Analytics.
## Issue Context
This PR appears to standardize filenames/imports to kebab-case, but the email-service event modules were not renamed (or shims were not added) to match the new require paths.
## Fix Focus Areas
- ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js[11-14]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const WellKnownController = require('./controllers/well-known-controller'); | ||
|
|
||
| const {EmailSuppressedEvent} = require('../../email-suppression-list/email-suppression-list'); | ||
| const MagicLink = require('../../lib/magic-link/magic-link'); |
There was a problem hiding this comment.
3. Magiclink require mismatch 🐞 Bug ✓ Correctness
• Members API now requires ../../lib/magic-link/magic-link, but the implementation present is services/lib/magic-link/MagicLink.js. • Other parts of the codebase still import MagicLink via the PascalCase path, reinforcing that the kebab-case module path is not valid. • Impact: loading Members API can fail at require-time, breaking magic-link-based member flows.
Agent prompt
## Issue description
`MembersAPI` requires `../../lib/magic-link/magic-link`, but the repo contains `services/lib/magic-link/MagicLink.js` and other code still imports `../lib/magic-link/MagicLink`. On case-sensitive filesystems this will throw `MODULE_NOT_FOUND` when Members API is loaded.
## Issue Context
This PR is performing kebab-case normalization, but MagicLink’s file path was updated without adding/renaming the underlying file.
## Fix Focus Areas
- ghost/core/core/server/services/members/members-api/members-api.js[18-20]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from agentic-review-benchmarks#7