Repository navigation
docs: add a language guide, and call a source's association data its member list - #21
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the member-list terminology update.
No open issues 🎉
What changed in this PR
Standardizes LaForge’s terminology across documentation, implementation, and specs, calling each source’s association data its “member list.” The changes preserve documented APIs and stored JSON formats.
Changes:
- Adds a language guide and links it from
CLAUDE.md. - Aligns internal names, error messages, examples, and spec descriptions with the guide.
- Updates README headings and links, merge-result terminology, and American spelling.
| File | Description |
|---|---|
spec/support/association_capabilities.rb |
Renames the fixture association to provided_parcel_links. |
spec/lib/usage_spec.rb |
Updates descriptions and expiry error expectations. |
spec/lib/association_importer_scenarios_spec.rb |
Aligns importer descriptions and fixture references. |
spec/lib/association_capabilities_spec.rb |
Updates terminology, local names, and error expectations. |
spec/lib/activerecord_spec.rb |
Updates a spec description to use “provided.” |
Readme.md |
Aligns explanations, examples, headings, and links. |
lib/laforge/tracked_association.rb |
Renames internal helpers and variables around member lists. |
lib/laforge/tombstone.rb |
Updates tombstone documentation terminology. |
lib/laforge/recording.rb |
Renames record_reported to record_pending and updates comments. |
lib/laforge/recording_settings.rb |
Aligns comments, examples, and error wording. |
lib/laforge/pending_writes.rb |
Updates comments to refer to member lists. |
lib/laforge/merge.rb |
Renames merge-result variables and member-list helpers. |
lib/laforge/member_list/partial.rb |
Aligns partial-list comments and local names. |
lib/laforge/member_list/complete.rb |
Aligns complete-list documentation. |
lib/laforge/member_list.rb |
Updates vocabulary and renames reported_keys to listed_keys. |
lib/laforge/inspection.rb |
Updates membership-expiry documentation. |
lib/laforge/forging.rb |
Updates helper names and pending-recording calls. |
lib/laforge/expiring.rb |
Aligns expiry comments and error wording. |
lib/laforge/association_config.rb |
Updates association comments, errors, and spelling. |
lib/laforge/activerecord.rb |
Updates the membership-expiry comment. |
CLAUDE.md |
Points contributors to the language guide. |
.ai/language.md |
Defines vocabulary, example-domain conventions, and writing rules. |
Review effort: Balanced
| - **People:** they/them for anyone whose pronouns you don't know. | ||
| - **Colons:** only to introduce a list, never inside a sentence. | ||
| - **Errors:** say what the developer has to declare or pass to fix it, naming the model, association or option. | ||
| - **README sections:** open with the common case. Put an unusual situation in a subsection named for the situation, such as "When members point at a column other than the owner's id". |
There was a problem hiding this comment.
"When members point at a column other than the owner's id". This is oddly specific but also doesn't give much extra information that couldn't be inferred from the instruction. Let's omit this.
There was a problem hiding this comment.
Removed in f5e14a8. The rule now ends at "Put an unusual situation in a subsection named for the situation."
| - **Colons:** only to introduce a list, never inside a sentence. | ||
| - **Errors:** say what the developer has to declare or pass to fix it, naming the model, association or option. | ||
| - **README sections:** open with the common case. Put an unusual situation in a subsection named for the situation, such as "When members point at a column other than the owner's id". | ||
| - **Code comments:** none by default. A class comment says what the class is for, as "Used to …" or a noun phrase defining the concept. A method comment starts with what the method returns. Define working vocabulary once, at the module that owns it. Never narrate a change. |
There was a problem hiding this comment.
What does "none by default" mean in this context? No code comments? Let's be clearer about whether this means inline comments or something else. I have no issue with comments, but we should not use them as a crutch when a better design or better method names could be used instead.
There was a problem hiding this comment.
Reworded in f5e14a8. It now says a comment explains what a clearer name or design can't, and that renaming the method or variable, or restructuring the code, comes first. It covers all comments, not just inline ones. It also adds the rule from Sparkle's comment guide: a class or module comment that's the first thing inside module LaForge opens with its name in backticks.
| if expires_on_source_id && source && expires_on_source_id == DataSource.normalize_source_id(source) | ||
| raise ArgumentError, "#{where}: expires_on: names the source recording, whose next recording replaces this one and its expiry anyway. " \ | ||
| 'Pass the source whose next report should end the change' | ||
| 'Pass the source whose next recording should end the change' |
There was a problem hiding this comment.
"source whose next recording" sounds awkward. What if instead of "record a source" we "record the data source". Let's discuss the use of language in this method's documentation.
There was a problem hiding this comment.
Settled on the language in the guide: a source provides values, and the application records values from a source. A source never records, so "the source's next recording" is gone. The error now reads "expires_on: names the source these values come from, and the next values recorded from it replace these and their expiry anyway. Pass another source, whose next values should end the change" (998cc41). In #22 the last part becomes "whose different values should end the change", since only a different value ends it there.
| # Entries marked for destruction are left out, so each of their attributes takes the value of the next entry, and an | ||
| # attribute with no other entry is left out of the hash. | ||
| # An attribute of a group is nil when the source that supplies the group has no entry for it. | ||
| # An attribute of a group is nil when the source that provides the group has no entry for it. |
There was a problem hiding this comment.
Does a source "provide the group"? Or is the source "providing data for the attributes in the group". Let's discuss.
There was a problem hiding this comment.
The source provides data for the group's attributes, not the group itself. The comment now uses your wording (998cc41): "A group's attributes all come from one source, so an attribute that source has no entry for is nil rather than falling back to another source's value". The README says the same.
| # list for the association. Members are named by key (see AssociationConfig). | ||
| # | ||
| # A report is one of: | ||
| # A member list is one of: |
There was a problem hiding this comment.
A member list is not one of these. The option takes one of these values. In fact, the documentation here does not follow the comment style guidelines. Let's discuss.
There was a problem hiding this comment.
Agreed. The class comment is rewritten (998cc41). It opens with "MemberList is one source's member list for a tracked association on one owner", then says it comes in two kinds, each a subclass, which implement the mode: option. It lists Complete (mode: :complete) and Partial (mode: :partial), and says mode: :edit changes a source's existing member list, so an edit is never presented as a kind of member list. The other class comments got the same rule-8 opening.
a1486f4 to
f4a091f
Compare
d7c0838 to
81fe371
Compare
…on on one Issue: comments, README text, spec descriptions and the `MemberList` comparison methods said a member list has an opinion on a member, which speaks of a data source as if it held views. The language guide now says a member list covers a member (#21). Instead of `MemberList#opinion` and `#same_opinions?`, the comparison methods are `#recorded_coverage` and `#same_recorded_coverage?`, since they count an expired inclusion or exclusion that no longer covers its member. `expired_on` takes `coverage_changed:` in place of `opinion_changed:`, and the C16 spec says an omitted attribute is not provided. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| | **tracked association** | A `has_many` or `has_one` whose membership LaForge forges from sources. | managed association | | ||
| | **owner**, **member** | The record a tracked association belongs to, which Rails calls its `owner`, and one of the association's records, which Rails calls an associated record. | parent, child, associated record | | ||
| | **member list** | What one source provides for a tracked association on one owner. A **complete list** covers every member, and a **partial list** only the members it names. An **edit** changes how a source's member list covers the members it names, and is never a kind of member list. | report, a bare "list" | | ||
| | a member list **covers** a member | It includes or excludes the member. Its **coverage** is which members it includes, which it excludes, and which it doesn't cover. An expired inclusion or exclusion no longer covers its member. | has an opinion, stance, view, verdict | |
There was a problem hiding this comment.
Right, fixed in 58d96e2. The row now says a partial list no longer covers a member once its inclusion or exclusion expires, while a complete list still excludes a member whose inclusion expired. The README bullet and the MemberList comment say the same.
8f362bf to
bb81e5f
Compare
Issue: LaForge had no record of which word names each concept, so the README and code mixed synonyms, such as a source that "reports" or "supplies" a value. `.ai/language.md` lists the term to use for each concept, the words to avoid, the example domain, and the writing rules. `CLAUDE.md` points to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nguage guide does Issue: `.ai/language.md` names what one source provides for a tracked association its member list (a complete list, a partial list or edits) and says a source provides values, but `lib/` and `spec/` still called it the source's "report", said sources "report" or "supply" values, and said a member list has an "opinion" on a member where the guide says it covers the member, and used British spellings such as "recognise", "behaviour" and "cancelled". Identifiers, comments, error messages and spec descriptions now use the guide's words and American spelling. `TrackedAssociation#report`/`#reports` become `#member_list`/`#member_lists`, `Forging#association_reports` becomes `#association_member_lists`, `MemberList#reported_keys` becomes `#listed_keys`, `Merge#combined_report` becomes `#combined_member_list`, `Recording#record_reported` becomes `#record_pending`, `TrackedAssociation#reported_columns` becomes `#member_columns`, `#recognise_tombstones` becomes `#recognize_tombstones`, and the hash `Merge#run` returns is built as `result`. Comments that said a member list has an opinion on a member now say it covers the member, and the C16 spec says an omitted attribute is not provided. The spec domain's `reported_parcel_links` scope becomes `provided_parcel_links`. Stored JSON keys are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… as the language guide does Issue: the README called what a source provides for a tracked association its "report", said sources "report" or "supply" values, called `LaForge.merge`'s return value a report, said a member list has an "opinion" on a member, and spelled "recognise" the British way, while `.ai/language.md` calls these a member list, provides, a result, and covers. The README now uses the guide's words, says once that members are Rails' associated records, and calls the merged association's outcome its membership, and says a member list covers a member. Headings are renamed to match, with every in-README link updated: "When a source stops reporting a record" is now "When a source stops providing a record", "Expiring an association report" is now "Expiring a member list", "Partial reports" is now "Partial lists", "Removed members that sources report again" is now "Removed members that sources list again", and "When a source stops reporting" is now "When a source stops listing members". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bb81e5f to
1ab662b
Compare
Adds
.ai/language.md, the words LaForge's README, comments, specs, errors, commits and PRs use, and brings the repo in line with it.Changes
.ai/language.mdgives the term for each concept, the words to avoid, the map-system example domain and the writing rules.CLAUDE.mdpoints to it.TrackedAssociation#report→#member_listandRecording#record_reported→#record_pending. No documented API changes, and the stored JSON is unchanged.LaForge.merge: its return value is called its result.#partial-reports→#partial-lists#expiring-an-association-report→#expiring-a-member-list#removed-members-that-sources-report-again→#removed-members-that-sources-list-again#when-a-source-stops-reporting-a-record→#when-a-source-stops-providing-a-record#when-a-source-stops-reporting→#when-a-source-stops-listing-membersrecognise_tombstones→recognize_tombstones.UAT Steps
bundle exec rspec: 403 examples, 0 failures, 2 pending.Special Handling
🤖 Generated with Claude Code