fix(telemetry): spell the property ghOwner, matching every other property - #194
Merged
Conversation
…erty Telemetry has two namespaces with two conventions, and they do not borrow from each other: events are snake_case (`cli_run`, `cli_check_completed`) and properties are camelCase (`cliVersion`, `scaffoldVersion`, `durationMs`, `loggedIn`, `ruleCount`, `errorCount`, `warningCount`). Every property followed that until `gh_owner`, which took the event spelling by mistake when the property was renamed from `gh_org` and nobody compared it to its neighbours. Correcting the one outlier rather than the eight properties around it. An earlier reading of the same evidence concluded the opposite and got as far as a reviewed pull request: renaming seven correct properties to match the single wrong one, at the cost of a permanent split in event history and a hand edit to every saved insight. Closed unmerged. No consumer migration. The property was added in this same unreleased cycle, so no stable build emitted `gh_owner` and nothing can be filtering on it. The `analytics` spec now states the convention normatively. Its absence is what allowed the mistake: with no rule to check against, one misspelled property read as evidence about the majority instead of as the outlier it was. The spec also records that PostHog's `$current_url` and `$lib_version` describe PostHog's own reserved namespace and are not guidance for ours, since that was the argument used to justify the wrong direction. Archived changes keep `gh_owner`, because that is what they shipped.
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
1 participant
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.
Renames the telemetry property
gh_ownertoghOwner, matching every other property.The convention was already there
Telemetry has two namespaces with two conventions, and neither follows the other:
snake_casecli_run,cli_check_completed,cli_rule_createdcamelCasecliVersion,scaffoldVersion,durationMs,loggedIn,ruleCount,errorCount,warningCountEvery property followed it.
gh_ownerwas added in #190 and took the event spelling by mistake, when the property was renamed fromgh_organd nobody compared it against its neighbours. It is the only property in the codebase spelled that way.Why this is the small PR and not the big one
The same evidence supported the opposite conclusion for a while. #193 read one
snake_caseproperty beside sevencamelCaseones and concluded the seven were wrong, proposing to rename them all. That would have moved the codebase away from its own convention and paid a permanent price for it: historical events keep their spellings forever, so every saved insight, dashboard and cohort filtering on those seven would have needed a hand edit, and any query spanning the release would need both names.Correcting the one outlier costs none of that. The property was added in this same unreleased cycle, so no stable build ever emitted
gh_ownerand nothing can be filtering on it. #193 is closed unmerged; #189 carries the full correction.The convention is now written down
The
analyticsspec gains a normative requirement stating the rule, with scenarios for adding an event, adding a property, and explicitly not borrowing one convention for the other.That absence is what allowed the mistake. With no rule to check against, a single misspelled property reads as evidence about the majority rather than as the outlier it is. The spec also records that PostHog's
$current_urland$lib_versiondescribe PostHog's own reserved namespace and are not guidance for this project's property names, because that was the argument used to justify the wrong direction.Archived changes keep
gh_owneropenspec/changes/archive/**is untouched. Those record what actually shipped, andgh_owneris what shipped in that change.Verification
pnpm typecheck,pnpm lint,pnpm test(929),pnpm cli check(exit 0),pnpm openspec validate --all --strict(24 passed).Refs #189
Refs #190