Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Rename $process_person to $process_person_profile #1143

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

robbie-c
Copy link
Member

@robbie-c robbie-c commented Apr 16, 2024

Changes

Rename $process_person to $process_person_profile

Checklist

  • Tests for new code (see advice on the tests we use)
  • Accounted for the impact of any changes across different browsers
  • Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

Copy link

Size Change: +32 B (0%)

Total Size: 962 kB

Filename Size Change
dist/array.full.js 230 kB +8 B (0%)
dist/array.js 127 kB +8 B (0%)
dist/es.js 128 kB +8 B (0%)
dist/module.js 128 kB +8 B (0%)
ℹ️ View Unchanged
Filename Size
dist/exception-autocapture.js 12.2 kB
dist/recorder-v2.js 108 kB
dist/recorder.js 108 kB
dist/surveys-module-previews.js 62 kB
dist/surveys.js 58.3 kB

compressed-size-action

@@ -903,7 +903,7 @@ export class PostHog {
}

// add person processing flag as very last step, so it cannot be overridden. process_person=true is default
properties['$process_person'] = this._hasPersonProcessing()
properties['$process_person_profile'] = this._hasPersonProcessing()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not exactly to do with this PR - but what does this mean for legacy SDKs. Will we all of a sudden not process persons because there is no $process_person_profile property?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a q for pipeline, but it should be that the absence of this property is treated as if it's true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We default to true, the only thing that disables it is sending $process_person_profile=false

@robbie-c robbie-c added the bump minor Bump minor version when this PR gets merged label Apr 16, 2024
@robbie-c robbie-c merged commit 3eb7fed into main Apr 16, 2024
15 checks passed
@robbie-c robbie-c deleted the feat/rename_process_person_to_process_person_profile branch April 16, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants