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.

Sign up for GitHub

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

Jump to bottom

Person and group created at for any order ingestion #7472

Merged
merged 10 commits into from
Jan 11, 2022
Merged

Person and group created at for any order ingestion #7472

merged 10 commits into from
Jan 11, 2022

Conversation

tiina303
Copy link
Contributor

@tiina303 tiina303 commented Dec 1, 2021 •
edited
Loading

Changes

Closes #7454

How did you test this code?

CI

@tiina303 tiina303 requested review from yakkomajuri and macobo and removed request for yakkomajuri December 2, 2021 17:31
@tiina303 tiina303 marked this pull request as ready for review December 2, 2021 18:26
@tiina303 tiina303 mentioned this pull request Dec 6, 2021
Closed
yakkomajuri
yakkomajuri reviewed Dec 7, 2021
View reviewed changes
Copy link
Contributor

@yakkomajuri yakkomajuri left a comment

Choose a reason for hiding this comment

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

looking good overall, a few comments

plugin-server/src/worker/ingestion/properties-updater.ts Show resolved Hide resolved
plugin-server/src/worker/ingestion/properties-updater.ts Show resolved Hide resolved
plugin-server/tests/clickhouse/postgres-parity.test.ts Outdated Show resolved Hide resolved
plugin-server/tests/clickhouse/postgres-parity.test.ts Show resolved Hide resolved
plugin-server/tests/shared/process-event.ts Show resolved Hide resolved
plugin-server/src/utils/db/db.ts Outdated Show resolved Hide resolved
@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@tiina303 tiina303 removed the stale label Dec 15, 2021
@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@tiina303 tiina303 removed the stale label Dec 29, 2021
@tiina303
Copy link
Contributor Author

tiina303 commented Jan 6, 2022

Some worker/ingestion tests are failing not sure if this pr or from before

@tiina303 tiina303 marked this pull request as draft January 6, 2022 00:35
yakkomajuri
yakkomajuri approved these changes Jan 6, 2022
View reviewed changes
Copy link
Contributor

@yakkomajuri yakkomajuri left a comment

Choose a reason for hiding this comment

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

overall lgtm, let's make sure tests pass

@tiina303 tiina303 marked this pull request as ready for review January 10, 2022 23:08
@tiina303 tiina303 merged commit bf59430 into master Jan 11, 2022
@tiina303 tiina303 deleted the person-creation branch January 11, 2022 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yakkomajuri yakkomajuri yakkomajuri approved these changes

@macobo macobo Awaiting requested review from macobo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Person created_at / first seen handling
3 participants
@tiina303 @posthog-bot @yakkomajuri