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

Release Persons V2 (persons-2353) #3461

Merged
merged 7 commits into from
Mar 2, 2021
Merged

Release Persons V2 (persons-2353) #3461

merged 7 commits into from
Mar 2, 2021

Conversation

paolodamico
Copy link
Contributor

@paolodamico paolodamico commented Feb 24, 2021 •
edited
Loading

Changes

Based on https://github.com/PostHog/retention/issues/15 we're releasing the new persons UI (persons-2353 feature flag) for everyone. This PR also removes all the legacy code.

Closes https://github.com/PostHog/retention/issues/15

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@paolodamico paolodamico changed the base branch from master to add-props February 24, 2021 20:07
@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 February 24, 2021 20:14 Inactive
@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 February 24, 2021 20:16 Inactive
@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 February 24, 2021 20:18 Inactive
@paolodamico
Copy link
Contributor Author

Need to merge #3394 first and then rebase for this PR to make sense. Will do that as soon as we merge the aforementioned PR.

@paolodamico paolodamico marked this pull request as draft February 24, 2021 20:21
@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 February 27, 2021 17:31 Inactive
@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 February 27, 2021 18:16 Inactive
@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 February 28, 2021 00:15 Inactive
Base automatically changed from add-props to master March 2, 2021 00:43
@paolodamico paolodamico removed the request for review from macobo March 2, 2021 00:44
@paolodamico paolodamico force-pushed the release-persons-2353 branch from 2b55992 to 205e919 Compare March 2, 2021 00:46
@paolodamico paolodamico requested a review from EDsCODE March 2, 2021 00:47
@paolodamico paolodamico marked this pull request as ready for review March 2, 2021 00:47
@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 March 2, 2021 00:47 Inactive
EDsCODE
EDsCODE reviewed Mar 2, 2021
View reviewed changes
Copy link
Member

@EDsCODE EDsCODE left a comment

Choose a reason for hiding this comment

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

Looks good, except I found a styling issue that somehow crept into this change. The labels in the radio button rectangles are not centered
Screen Shot 2021-03-02 at 9 30 45 AM

@EDsCODE EDsCODE mentioned this pull request Mar 2, 2021
Closed
4 tasks
@paolodamico
Copy link
Contributor Author

Thanks for the review, fixed. Ready for one final look if you want to take a look, will merge a bit later today otherwise.

@timgl timgl temporarily deployed to posthog-release-persons-3ftil8 March 2, 2021 16:03 Inactive
@paolodamico paolodamico merged commit 7e10a4f into master Mar 2, 2021
@paolodamico paolodamico deleted the release-persons-2353 branch March 2, 2021 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EDsCODE EDsCODE EDsCODE left review comments

@yakkomajuri yakkomajuri Awaiting requested review from yakkomajuri

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

Successfully merging this pull request may close these issues.
3 participants
@paolodamico @EDsCODE @timgl