Removed xmlrpc/pingomatic ping service#12
Conversation
Pingomatic is a legacy blog ping service that is no longer relevant for modern SEO or content distribution. The service was disabled by default and added unnecessary complexity to the codebase.
WalkthroughThis pull request removes the XML-RPC service, including the service module, its initialization in the boot process, its configuration option, and all associated tests. The changes clean up dependencies and simplify service initialization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ghost/core/core/boot.js (1)
349-365: Fix race condition: emailService.init() requires emailAddressService to complete initialization first.The concurrent execution of
emailAddressService.init()andemailService.init()in the samePromise.all()introduces a real race condition. In EmailServiceWrapper.init(), both EmailRenderer and SendingService are constructed withemailAddressService.servicepassed as a dependency. IfemailService.init()executes beforeemailAddressService.init()completes,emailAddressService.servicewill be undefined. These services use emailAddressService during email rendering (ingetFromAddress()andgetReplyToAddress()methods), so passing undefined will cause failures downstream.Move
emailAddressService.init()outside of thePromise.all()and execute it beforeemailService.init()to guarantee the dependency is satisfied, or add an explicit await to ensureemailAddressService.init()completes before startingemailService.init().
🤖 Fix all issues with AI agents
In `@ghost/core/core/boot.js`:
- Line 319: Remove the unused slack import by deleting the const slack =
require('./server/services/slack'); statement and any unused references to the
local variable 'slack' in boot.js; if the intent was to register listeners,
either call slack.listen() from the appropriate initialization sequence
(ensuring the 'listen' function on the slack module registers 'post.published'
and 'slack.test') or drop the import entirely so there are no dead requires
remaining.
- Line 368: The scheduling.init() call is invoking PostScheduler without the
required apiUrl, causing PostScheduler (PostScheduler.js) to throw
errors.IncorrectUsageError; fix by passing the API URL into
scheduling.init(options) from boot (e.g. obtain the URL from config or urlUtils
where boot can access it) so scheduling.init receives {apiUrl: <siteApiUrl>} and
forwards it to the PostScheduler constructor, or alternatively modify the
scheduling adapter to read the API URL from configuration (config.get or
urlUtils) inside PostScheduler instead of requiring it as a constructor
parameter.
🧹 Nitpick comments (1)
ghost/core/core/boot.js (1)
313-313: Consider relocating this developer note.This comment about using
npm installfor service dependencies seems more appropriate in a README or CONTRIBUTING guide rather than inline in the boot sequence code. If the intent is to warn developers about a specific quirk, consider adding more context.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ghost/core/core/boot.jsghost/core/core/server/services/xmlrpc.jsghost/core/core/shared/config/env/config.development.jsonghost/core/test/unit/server/services/xmlrpc.test.js
💤 Files with no reviewable changes (3)
- ghost/core/test/unit/server/services/xmlrpc.test.js
- ghost/core/core/server/services/xmlrpc.js
- ghost/core/core/shared/config/env/config.development.json
🔇 Additional comments (1)
ghost/core/core/boot.js (1)
351-378: LGTM - Service initialization batch looks well-structured.The concurrent initialization of independent services using
Promise.allis an appropriate pattern for boot performance. The service dependencies (stripe before members, etc.) appear to be correctly ordered withstripe.init()awaited before the parallel batch.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const tiers = require('./server/services/tiers'); | ||
| const permissions = require('./server/services/permissions'); | ||
| const xmlrpc = require('./server/services/xmlrpc'); | ||
| const slack = require('./server/services/slack'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if slack module is used in boot.js
echo "=== Searching for slack usage in boot.js ==="
rg -n 'slack' ghost/core/core/boot.js | head -20
echo ""
echo "=== Checking for slack module file ==="
find . -path "*/services/slack*" -type f 2>/dev/null | head -10Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 988
🏁 Script executed:
#!/bin/bash
# Get more context around line 319 and the mentioned range (351-378)
echo "=== Context around line 319 (import) ==="
sed -n '310,330p' ghost/core/core/boot.js
echo ""
echo "=== Context around lines 351-378 ==="
sed -n '345,385p' ghost/core/core/boot.jsRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 2589
🏁 Script executed:
#!/bin/bash
# Check if slack module has side effects on require
if [ -f "ghost/core/core/server/services/slack/index.js" ]; then
echo "=== Slack module implementation (first 60 lines) ==="
head -60 ghost/core/core/server/services/slack/index.js
else
echo "Slack module not found at expected location, searching for it..."
find . -name "slack" -type d 2>/dev/null | head -5
find . -path "*slack/index.js" 2>/dev/null | head -5
fiRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Read the slack.js file to understand what it does
echo "=== Content of slack.js ==="
cat ghost/core/core/server/services/slack.js
echo ""
echo "=== Verify slack is not used elsewhere in codebase ==="
# Search for usage of the slack variable (excluding the import line)
rg -n 'slack\.' --type=js | grep -v 'slack-notifications' | grep -v '.test.js' | head -20Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 8286
Remove unused slack import.
The slack service imported on line 319 is never called in the initialization sequence. The module's listen() function is not invoked anywhere in boot.js, so the event listeners for 'post.published' and 'slack.test' are never registered. This import should be removed as it serves no purpose.
🤖 Prompt for AI Agents
In `@ghost/core/core/boot.js` at line 319, Remove the unused slack import by
deleting the const slack = require('./server/services/slack'); statement and any
unused references to the local variable 'slack' in boot.js; if the intent was to
register listeners, either call slack.listen() from the appropriate
initialization sequence (ensuring the 'listen' function on the slack module
registers 'post.published' and 'slack.test') or drop the import entirely so
there are no dead requires remaining.
| scheduling.init({ | ||
| apiUrl: urlUtils.urlFor('api', {type: 'admin'}, true) | ||
| }), | ||
| scheduling.init(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the scheduling adapter's init function signature and URL resolution
echo "=== Checking scheduling adapter init signature ==="
find ghost -name "*.js" -path "*scheduling*" | head -20
echo ""
echo "=== Checking init method in scheduling adapter ==="
rg -n "function init\|\.init\s*=" ghost/core/core/server/adapters/scheduling/ -A 5 | head -50Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 224
🏁 Script executed:
#!/bin/bash
# Search for how scheduling adapter handles API URL resolution
echo "=== Search for apiUrl references in scheduling adapter ==="
rg -n "apiUrl" ghost/core/core/server/adapters/scheduling/ -C 3
echo ""
echo "=== Check for config or urlUtils usage in scheduling adapter ==="
rg -n "(config|urlUtils)" ghost/core/core/server/adapters/scheduling/ | head -30Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 10760
🏁 Script executed:
#!/bin/bash
# Check the boot.js file to understand the context of scheduling.init() call
echo "=== Context around line 368 in boot.js ==="
sed -n '360,375p' ghost/core/core/boot.jsRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 612
Scheduling service will fail - apiUrl parameter must be provided but is missing.
The scheduling adapter's PostScheduler constructor explicitly requires an apiUrl parameter and throws an error if not provided (see PostScheduler.js line 10: throw new errors.IncorrectUsageError({message: 'post-scheduling: no apiUrl was provided'})). Calling scheduling.init() with no arguments will pass undefined, causing a runtime failure. The adapter does not have fallback logic to resolve the API URL internally—it must be passed explicitly. Either pass the apiUrl from boot.js to scheduling.init(options) or refactor the scheduling adapter to resolve the URL internally (via config or urlUtils).
🤖 Prompt for AI Agents
In `@ghost/core/core/boot.js` at line 368, The scheduling.init() call is invoking
PostScheduler without the required apiUrl, causing PostScheduler
(PostScheduler.js) to throw errors.IncorrectUsageError; fix by passing the API
URL into scheduling.init(options) from boot (e.g. obtain the URL from config or
urlUtils where boot can access it) so scheduling.init receives {apiUrl:
<siteApiUrl>} and forwards it to the PostScheduler constructor, or alternatively
modify the scheduling adapter to read the API URL from configuration (config.get
or urlUtils) inside PostScheduler instead of requiring it as a constructor
parameter.
2 participants
Benchmark PR from qodo-benchmark#226
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.