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

remove news.featured property #3380

Merged
merged 8 commits into from Sep 17, 2019
Merged

remove news.featured property #3380

merged 8 commits into from Sep 17, 2019

Conversation

Pomax
Copy link
Contributor

@Pomax Pomax commented Jul 9, 2019

Closes #3379
Related PRs/issues #721, #3296

The last time we landed this we ran into a caching/desync issue where the old code was using the updated db and things crashed because the old code was trying to load a column that the new db didn't have anymore...

I don't know if that's still going to be the case, but last time we discovered that rather than rolling back the code change, we could have waited 2 minutes for things to sync back up, so.... maybe we'll have to do this again when we deploy?

@Pomax Pomax requested a review from patjouk July 9, 2019 22:32
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-3380 July 9, 2019 22:32 Inactive
@Pomax Pomax temporarily deployed to foundation-mofostaging-pr-3380 July 10, 2019 16:08 Inactive
@Pomax Pomax requested review from gideonthomas and removed request for patjouk July 25, 2019 18:46
@Pomax Pomax added this to the Aug 5 milestone Jul 25, 2019
@patjouk patjouk self-requested a review July 29, 2019 10:10
Copy link
Contributor

@patjouk patjouk left a comment

Choose a reason for hiding this comment

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

Is this field still used anywhere in the code? If yes, that's when you will get 5xx. If you already cleaned up the code, make sure it's already deployed before merging this one.
If you have doubts, you can checks the docs I wrote about migrations here: https://github.com/mozilla/foundation.mozilla.org/blob/master/docs/workflow.md#django-migrations-what-to-do-when-working-on-backward-incompatible-migrations

@Pomax
Copy link
Contributor Author

Pomax commented Jul 29, 2019

Yeah, I'm pretty sure we're not using it anywhere anymore, which is why I'm hopeful it won't be a problem this time round anymore. But that template update was a great idea!

@Pomax Pomax modified the milestones: Aug 5, Aug 19 Aug 6, 2019
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-3380 August 7, 2019 20:44 Inactive
@Pomax Pomax merged commit 73d38ae into master Sep 17, 2019
@Pomax Pomax deleted the remove-news-feature-flag branch September 17, 2019 01:03
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.

Remove featured property from News model
4 participants