Renamed files to kebab-case - core - services - part 2#22
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
• members-api.js includes multiple new const ... = require(...) statements without terminating semicolons, relying on ASI. • This violates the requirement that all JavaScript statements end with explicit semicolons, and introduces inconsistency since later require lines in the same file do use semicolons. • This can lead to subtle parsing issues in certain edge cases and reduces formatting consistency across the codebase.
Agent prompt
## Issue description
The file `members-api.js` has multiple `const ... = require(...)` statements without trailing semicolons, relying on automatic semicolon insertion (ASI), which violates the semicolon requirement and is inconsistent with other imports in the same file.
## Issue Context
Compliance requires explicit semicolons for all JS/TS statements.
## 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
| 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.
2. Magiclink require mismatch 🐞 Bug ✓ Correctness
• Members API now requires a lowercase path for MagicLink, but the implementation file in that directory is still named MagicLink.js (PascalCase). • On case-sensitive filesystems, this will throw MODULE_NOT_FOUND when Members API is initialized. • This is inconsistent with other call sites (e.g. newsletters) which still require ../lib/magic-link/MagicLink, indicating the file was not actually renamed in this PR branch.
Agent prompt
### Issue description
`MembersAPI` requires `../../lib/magic-link/magic-link`, but the implementation file is `MagicLink.js`. On case-sensitive systems this will cause `MODULE_NOT_FOUND`.
### Issue Context
Other code (e.g. newsletters) still requires `../lib/magic-link/MagicLink`, strongly suggesting the file was not renamed in this branch.
### Fix Focus Areas
- ghost/core/core/server/services/members/members-api/members-api.js[18-20]
- ghost/core/core/server/services/newsletters/newsletters-service.js[8-10]
- ghost/core/core/server/services/lib/magic-link/MagicLink.js[1-4]
### Suggested fix
Option A (minimal): change the Members API import to `require('../../lib/magic-link/MagicLink')`.
Option B (complete rename): rename `MagicLink.js` to `magic-link.js` (or add a `magic-link.js` re-export) and update *all* `MagicLink` requires accordingly.
ⓘ 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.
3. Email analytics requires broken 🐞 Bug ✓ Correctness
• EmailAnalyticsServiceWrapper now requires ../email-service/email-event-storage and ../email-service/email-event-processor. • The email-service implementations in this branch are still EmailEventStorage.js and EmailEventProcessor.js (PascalCase). • This will crash email analytics initialization on case-sensitive filesystems, disabling analytics jobs/event processing.
Agent prompt
### Issue description
`EmailAnalyticsServiceWrapper` requires `../email-service/email-event-storage` and `../email-service/email-event-processor`, but the existing email-service files are `EmailEventStorage.js` and `EmailEventProcessor.js`. This will cause runtime failures on case-sensitive systems.
### Issue Context
This PR is standardizing filenames/paths. Either the email-service files need to be renamed too, or the wrapper should keep requiring the existing PascalCase files.
### Fix Focus Areas
- ghost/core/core/server/services/email-analytics/email-analytics-service-wrapper.js[11-14]
- ghost/core/core/server/services/email-service/EmailEventStorage.js[1-6]
- ghost/core/core/server/services/email-service/EmailEventProcessor.js[1-9]
### Suggested fix
Option A (minimal): change the requires back to:
- `require('../email-service/EmailEventStorage')`
- `require('../email-service/EmailEventProcessor')`
Option B (complete rename): rename the email-service files to `email-event-storage.js` / `email-event-processor.js` and update any other references consistently.
ⓘ 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