Skip to content

LG-12304 Remove unused phone confirmation data (2 of 2)#12013

Merged
kevinsmaster5 merged 5 commits intomainfrom
kmas-lg-12304-remove-unused-phone-conf-db
Apr 2, 2025
Merged

LG-12304 Remove unused phone confirmation data (2 of 2)#12013
kevinsmaster5 merged 5 commits intomainfrom
kmas-lg-12304-remove-unused-phone-conf-db

Conversation

@kevinsmaster5
Copy link
Copy Markdown
Contributor

@kevinsmaster5 kevinsmaster5 commented Mar 21, 2025

🎫 Ticket

Link to the relevant ticket:
LG-12304

For 50/50 state expands previous pr #12012

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@vrajmohan should I move this change to Part 1 #12012 I'm looking at how you're conducting a similar ticket.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Then I think it gets removed with this PR if I'm understanding the order of operations.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@vrajmohan vrajmohan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@vrajmohan This was causing errors due to confirmed_at no longer being available here. I'm thinking I should split this and the test changes off into another pull, then make the DB changes in p3. Does that sound right?

@vrajmohan
Copy link
Copy Markdown
Contributor

vrajmohan commented Mar 27, 2025

I'm confused - why did we not ignore confirmed_at (ie add it to self.ignored_columns) in the previous commit?

@vrajmohan
Copy link
Copy Markdown
Contributor

@kevinsmaster5
Copy link
Copy Markdown
Contributor Author

I'm confused - why did we not ignore confirmed_at (ie add it to self.ignored_columns) in the previous commit?

You are correct on both counts.
I have an amended PR to remedy what I needed in that PR
#12029

@kevinsmaster5 kevinsmaster5 force-pushed the kmas-lg-12304-remove-unused-phone-conf-db branch 2 times, most recently from 4551852 to 00a821b Compare March 31, 2025 16:38
@kevinsmaster5 kevinsmaster5 force-pushed the kmas-lg-12304-remove-unused-phone-conf-db branch from 00a821b to e11b62d Compare April 2, 2025 13:16
@kevinsmaster5 kevinsmaster5 marked this pull request as ready for review April 2, 2025 13:16
@kevinsmaster5
Copy link
Copy Markdown
Contributor Author

@vrajmohan nothing much has changed since you reviewed but thought I'd give you a nudge before merging later today.

@kevinsmaster5 kevinsmaster5 merged commit c28ade5 into main Apr 2, 2025
1 check passed
@kevinsmaster5 kevinsmaster5 deleted the kmas-lg-12304-remove-unused-phone-conf-db branch April 2, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants