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

Update UserGroupEvent details format and log interface #9487

Merged
merged 7 commits into from
Jan 5, 2023
Merged

Update UserGroupEvent details format and log interface #9487

merged 7 commits into from
Jan 5, 2023

Conversation

cl8n
Copy link
Member

@cl8n cl8n commented Nov 9, 2022 •
edited by nanaya
Loading

@cl8n cl8n mentioned this pull request Nov 9, 2022
Merged
1 task
nanaya
nanaya requested changes Dec 2, 2022
View reviewed changes
app/Models/UserGroupEvent.php Outdated Show resolved Hide resolved
database/migrations/2022_11_06_022852_require_user_group_events_details_and_type.php Outdated
Comment on lines 21 to 23
// The format of `details` before this was not finalised, and on
// production nothing important was logged anyway. No going back!
UserGroupEvent::truncate();
Copy link
Collaborator

@nanaya nanaya Dec 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

note that there may still be old events come in after the migration because the migration is run before code deploy

Copy link
Member Author

Choose a reason for hiding this comment

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

true, should I change anything here to account for that? or just ask peppy to truncate again after deploy?

nanaya
nanaya approved these changes Jan 5, 2023
View reviewed changes
Copy link
Collaborator

@nanaya nanaya left a comment

Choose a reason for hiding this comment

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

@peppy please run the migration eventually. The exact timing shouldn't matter. It's just dropping existing data and adding null check to the columns which should already be always filled in.

@nanaya nanaya merged commit 58f5229 into ppy:master Jan 5, 2023
@cl8n cl8n deleted the user-group-event-update branch May 19, 2023 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nanaya nanaya nanaya approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@cl8n @nanaya