Removed xmlrpc/pingomatic ping service#38
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.
WalkthroughThe PR removes the XMLRPC service entirely from the Ghost core, including its initialization from the boot sequence, the service implementation file, development configuration settings, and corresponding unit tests. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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: Potential race condition:emailAddressService.init()should complete beforeemailService.init().The comment at line 349 explicitly states that "newsletter service and email service depend on email address service." However,
emailAddressService.init()is now insidePromise.allalongsideemailService.init(), meaning they run in parallel with no ordering guarantee.If
emailService.init()executes beforeemailAddressService.init()completes, this could cause initialization failures or undefined behavior.Proposed fix: await emailAddressService.init() before Promise.all
// NOTE: newsletter service and email service depend on email address service + await emailAddressService.init(); await Promise.all([ - emailAddressService.init(), identityTokens.init(), memberAttribution.init(),
🤖 Fix all issues with AI agents
In `@ghost/core/core/boot.js`:
- Line 319: Remove the dead import of the Slack service: delete the line that
requires the slack module (const slack = require('./server/services/slack');)
since slack is never used and slack.listen() was removed; ensure no other
references to the slack symbol remain in this file (boot.js) and run a quick
search to confirm there are no remaining unused variables introduced by this
change.
- Line 368: scheduling.init() is being called without the required apiUrl
causing PostScheduler to throw IncorrectUsageError; update the boot sequence to
pass the configured API URL into scheduling.init(apiUrl). Locate the call to
scheduling.init() and the PostScheduler constructor usage, obtain the
application's API URL from the existing config object (or equivalent env value
used elsewhere for JWT/webhook URL construction), and call
scheduling.init(config.apiUrl) so JWT token generation and webhook callback URLs
have the needed parameter.
🧹 Nitpick comments (1)
ghost/core/core/boot.js (1)
313-313: Comment seems misplaced or overly generic.This comment about npm install appears out of context in the services initialization section. Consider removing it or placing it in a more relevant location (e.g., a README or contribution guide).
📜 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/core/server/services/xmlrpc.js
- ghost/core/core/shared/config/env/config.development.json
- ghost/core/test/unit/server/services/xmlrpc.test.js
✏️ 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.
Unused import: slack is imported but never used.
The slack service is imported but not called anywhere in this file. According to the summary, slack.listen() was removed along with the XMLRPC service. This import is now dead code and should be removed.
Proposed fix
- const slack = require('./server/services/slack');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const slack = require('./server/services/slack'); |
🤖 Prompt for AI Agents
In `@ghost/core/core/boot.js` at line 319, Remove the dead import of the Slack
service: delete the line that requires the slack module (const slack =
require('./server/services/slack');) since slack is never used and
slack.listen() was removed; ensure no other references to the slack symbol
remain in this file (boot.js) and run a quick search to confirm there are no
remaining unused variables introduced by this change.
| scheduling.init({ | ||
| apiUrl: urlUtils.urlFor('api', {type: 'admin'}, true) | ||
| }), | ||
| scheduling.init(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find scheduling adapter files
echo "=== Finding scheduling adapter files ==="
fd -t f -e js . | grep -i scheduling | head -20
echo -e "\n=== Checking scheduling service structure ==="
fd -t f 'index.js' -o 'adapter.js' | grep -i scheduling
echo -e "\n=== Search for init function in scheduling ==="
rg -l 'init.*function|export.*init' --type js | grep -i schedulingRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 1328
🏁 Script executed:
#!/bin/bash
# Check scheduling adapter's init implementation
echo "=== Searching for scheduling init function ==="
rg -n -B2 -A10 'function init|init.*=.*function|const init.*=' --type js | grep -i -A12 scheduling
echo -e "\n=== Searching for apiUrl usage in scheduling context ==="
rg -n -C3 'apiUrl' --type js | grep -i -B3 -A3 scheduling
echo -e "\n=== Check for scheduling adapter exports ==="
rg -n 'module.exports|export' --type js | grep -i schedulingRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 16890
🏁 Script executed:
#!/bin/bash
# Look at the scheduling service files directly
echo "=== Find all scheduling related files ==="
find . -type f -name "*.js" -path "*scheduling*" 2>/dev/null
echo -e "\n=== Search for any scheduling adapter definition ==="
rg -l "scheduling" --type js | head -10Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 1637
🏁 Script executed:
#!/bin/bash
# Check the boot.js file around line 368
echo "=== boot.js around line 368 ==="
sed -n '350,385p' ghost/core/core/boot.js
echo -e "\n=== Check git history for scheduling.init changes ==="
cd ghost/core && git log --oneline -n 20 -- core/boot.js | head -10
cd - > /dev/null
echo -e "\n=== Search for where apiUrl should come from ==="
rg -n 'scheduling\.init' --type jsRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 1239
🏁 Script executed:
#!/bin/bash
# Check git diff to see what changed in this PR
echo "=== Git diff for boot.js ==="
cd ghost/core && git show HEAD:core/boot.js | sed -n '350,385p'
cd - > /dev/null
echo -e "\n=== Check if there's an apiUrl available in the boot context ==="
rg -n -B5 -A5 'scheduling\s*=' ghost/core/core/boot.js | head -40
echo -e "\n=== Check how scheduling service is initialized elsewhere ==="
rg -n -B3 -A3 'scheduling' ghost/core/core/boot.js | head -50Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 1852
🏁 Script executed:
#!/bin/bash
# Look at the git diff for boot.js
cd ghost/core && git diff HEAD~1 core/boot.js || git diff HEAD core/boot.js | head -100
cd - > /dev/nullRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 270
🏁 Script executed:
#!/bin/bash
# Look for where apiUrl should come from in boot context
echo "=== Search for apiUrl in boot.js context ==="
rg -n -B10 'scheduling' ghost/core/core/boot.js | grep -E '(apiUrl|getBaseUrl|config)' | head -20
echo -e "\n=== Check if there's a config or utils that provides apiUrl ==="
rg -n 'apiUrl.*=' ghost/core/core/boot.js
echo -e "\n=== Look for getAdminUrl or similar URL builders ==="
rg -n 'getAdminUrl|getBaseUrl|apiUrl' ghost/core/core/boot.js | head -20
echo -e "\n=== Check what config contains ==="
sed -n '1,100p' ghost/core/core/boot.js | grep -E '(config|require|const)'Repository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 1137
🏁 Script executed:
#!/bin/bash
# Search for how other services get apiUrl or if there's a pattern
echo "=== Check other init patterns that might need URL ==="
rg -n -B2 'webhooks\.listen|emailService\.init' ghost/core/core/boot.js
echo -e "\n=== Look for apiUrl or admin URL elsewhere in boot ==="
rg -n 'config\..*Url|getAdminUrl|apiUrl' ghost/core/core/boot.js
echo -e "\n=== Check if there's documentation about scheduling init ==="
rg -n -B5 'scheduling' ghost/core/core/server/adapters/scheduling/index.jsRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 763
🏁 Script executed:
#!/bin/bash
# Check the post-scheduling init function signature more carefully
echo "=== Detailed look at post-scheduling/index.js init function ==="
sed -n '33,50p' ghost/core/core/server/adapters/scheduling/post-scheduling/index.js
echo -e "\n=== Check the PostScheduler constructor signature ==="
sed -n '8,12p' ghost/core/core/server/adapters/scheduling/post-scheduling/PostScheduler.js
echo -e "\n=== Check if apiUrl can be retrieved from somewhere ==="
rg -n 'config\.getSiteUrl\|config\.getAdminUrl' ghost/core/core/server/adapters/scheduling/ ghost/core/core/server/services/
echo -e "\n=== Check what functions are exported from scheduling adapter ==="
cat ghost/core/core/server/adapters/scheduling/index.jsRepository: qodo-benchmark/Ghost-combined-coderabbit
Length of output: 1380
scheduling.init() requires the apiUrl parameter and will throw an error without it.
The PostScheduler constructor explicitly validates that apiUrl is provided and throws an IncorrectUsageError with message "post-scheduling: no apiUrl was provided" if it is missing. The apiUrl is not derived internally—it is required for JWT token generation and constructing webhook callback URLs. Calling scheduling.init() without arguments will cause the application to fail during boot.
🤖 Prompt for AI Agents
In `@ghost/core/core/boot.js` at line 368, scheduling.init() is being called
without the required apiUrl causing PostScheduler to throw IncorrectUsageError;
update the boot sequence to pass the configured API URL into
scheduling.init(apiUrl). Locate the call to scheduling.init() and the
PostScheduler constructor usage, obtain the application's API URL from the
existing config object (or equivalent env value used elsewhere for JWT/webhook
URL construction), and call scheduling.init(config.apiUrl) so JWT token
generation and webhook callback URLs have the needed parameter.
2 participants
Benchmark PR from agentic-review-benchmarks#1
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.