Correct documentation drift against the current implementation - #94
Merged
Conversation
Verified every documented claim against source and fixed the ones that no longer held: - CONFIG.md: assignmentAlertEnabled, welcomeMessageEnabled, and quotesEnabled described behavior the code does not have - COMMANDS.md, USER_GUIDE.md: /m open, /m delete, and /m archive require a message ID; /m config takes show or set - API.md: getMailbox and getMessage always return a wrapper, getMessage only searches active messages, and the M_Mailbox mutators were undocumented - ATTACHMENTS.md: no max-attachments option exists, only a stack size cap - API.md, QUICKSTART.md: dependency examples referenced 1.2.0, not 1.3.0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The getMessage example caught an NPE to detect a missing message, which teaches integrating plugins a bad pattern. It now shows the direct call alongside M_Mailbox.getActiveMessage, which does return null when absent. The CHANGELOG entry is also split into readable bullets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…bsent Mailbox.removeActiveMessage(int) and removeArchivedMessage(int) look the message up first and pass null through to List.remove when nothing matches, so neither throws. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
Self-review rubric
Findings folded in from the diff read
This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). |
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
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.
Summary
A repo-wide documentation accuracy sweep was performed: every claim in the documentation sources of truth was checked against the code, config, and commands it documents, and the claims that no longer held were corrected. No production code was changed — where the code was found to be at fault, an issue was filed instead (see below).
Corrections made:
CONFIG.md— three option descriptions did not match the code.assignmentAlertEnabledwas described as notifying a player when a message is sent to them, but it gates the "You have been assigned a mailbox" alert emitted the first time a mailbox is created (MailboxService.assignMailboxToPlayerIfNecessary).welcomeMessageEnabledwas described as a message shown on join, but a welcome message is delivered to the mailbox only when one is first created.quotesEnabledwas described as displaying random quotes; it wraps message content in double quotes when a message is opened (Message.sendContentToPlayer).COMMANDS.md/USER_GUIDE.md—/m open,/m delete, and/m archivewere listed without their required message-ID argument, and/m configwas listed without itsshow/set <option> <value>sub-commands. The accepted/m listtypes (active,archived,unread) and the double-quote requirement on/m sendare now stated.API.md—getMessage(int)was documented as returningnullwhen not found; a non-null wrapper aroundnullis always returned, so the documentednullguard never fires. The same applies togetMailbox(Player). It is also now recorded thatgetMessage(int)searches active messages only, that the twosendListOf...methods send only the first page, and that the eightM_Mailboxmutators exist — they were entirely absent from the reference.ATTACHMENTS.md— a "max attachments" config option was claimed; no such option exists, onlymaxAttachmentStackSize.API.md/QUICKSTART.md— dependency examples pinned1.2.0whilepom.xmlis at1.3.0.CHANGELOG.md— an entry was added under[Unreleased].Issues filed rather than fixed here
Per the docs-sweep rule that code must not be silently changed under a documentation cycle, three code-level findings were filed:
/m helpoutput and theConfigCommand/SendCommandusage strings do not match the accepted syntax (/c config set ..., single quotes on/m send, missing ID arguments).MailboxesAPI.getMailbox/getMessagereturn wrappers aroundnull, so integrating plugins get aNullPointerExceptionat a later call site instead of anullreturn.Backlog deferred this cycle
Test plan
mvn test— 41 tests, 0 failures, 0 errors, BUILD SUCCESSMailboxService,Message.sendContentToPlayer,CommandInterpreter, thecommands/usage strings,ConfigService,MailboxesAPI,M_Mailbox,Mailbox,PersistentData,SendCommand, andpom.xmlThis PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).