Skip to content

Rename values in SiteConfiguration/History (1/3) #22692

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
Merged
doctoryes merged 1 commit into from
Jan 14, 2020
Merged

Rename values in SiteConfiguration/History (1/3) #22692

doctoryes merged 1 commit into from
Jan 14, 2020

Conversation

@doctoryes
Copy link
Contributor

@doctoryes doctoryes commented Jan 2, 2020 •
edited
Loading

https://openedx.atlassian.net/browse/DE-1826

This stage does the following:

  • Adds the new field and migration to create the column.
  • Makes all writes go to both old and new field.

Stage 1: this PR.
Stage 2: #22709
Stage 3: #22753

Testing plan detailed in https://openedx.atlassian.net/wiki/spaces/DE/pages/1065681172/Patterns+For+Renaming+Django+Model+Fields#PatternsForRenamingDjangoModelFields-RolloutPlanForCreditRequirement.orderIntheLMS

@doctoryes doctoryes requested review from a team January 2, 2020 22:11
@doctoryes doctoryes requested a review from a team as a code owner January 2, 2020 22:11
@doctoryes doctoryes requested a review from a team January 2, 2020 22:11
@doctoryes doctoryes force-pushed the juliasq/rename_site_configuration_siteconfiguration_values_1 branch from 9bd4e92 to e542cd9 Compare January 3, 2020 17:05
This was referenced Jan 3, 2020
Closed
Closed
Merged
bmedx
bmedx approved these changes Jan 13, 2020
View reviewed changes
Copy link
Contributor

@bmedx bmedx left a comment

Choose a reason for hiding this comment

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

Seems fine to me. Only nit is that some places have TODOs and others don't. Not a big deal since presumably 2/3 and 3/3 will be hot on the heels of this one.

@doctoryes doctoryes force-pushed the juliasq/rename_site_configuration_siteconfiguration_values_1 branch from e542cd9 to 84ca5af Compare January 13, 2020 22:46
This stage does the following:
05f2e39 
- Adds the new field and migration to create the column.
- Makes all writes go to both old and new field.
@doctoryes doctoryes force-pushed the juliasq/rename_site_configuration_siteconfiguration_values_1 branch from 84ca5af to 05f2e39 Compare January 14, 2020 15:10
@edx-status-bot
Copy link

edx-status-bot commented Jan 14, 2020

Your PR has finished running tests. There were no failures.

@doctoryes doctoryes merged commit cebeab4 into master Jan 14, 2020
doctoryes pushed a commit that referenced this pull request Jan 14, 2020
Revert "This stage does the following: (#22692)"
84de6bc 
This reverts commit cebeab4.
@doctoryes doctoryes mentioned this pull request Jan 14, 2020
Merged
@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jan 14, 2020

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jan 14, 2020

EdX Release Notice: This PR may have caused e2e tests to fail on Stage. If you're a member of the edX org, please visit #e2e-troubleshooting on Slack to help diagnose the cause of these failures. Otherwise, it is the reviewer's responsibility. E2E tests have failed. https://gocd.tools.edx.org/go/tab/pipeline/history/STAGE_edxapp_M-D

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jan 21, 2020

EdX Release Notice: This PR has been deployed to the production environment.

pwnage101 added a commit that referenced this pull request Feb 19, 2020
@pwnage101
Revert "Revert "This stage does the following: (#22692)""
18deacd 
This reverts commit 84de6bc
which reverts commit cebeab4
which implements the first stage of the `values` column rename in
SiteConfiguration.  However, I included a small change:

This time, we set a default value on the new `site_values` column so
that the ORM will happily deserialize the JSONField without throwing a
JSONDecodeError.
@pwnage101 pwnage101 mentioned this pull request Feb 19, 2020
Merged
@Agrendalath Agrendalath mentioned this pull request Jun 9, 2020
Closed
@kdmccormick kdmccormick deleted the juliasq/rename_site_configuration_siteconfiguration_values_1 branch July 16, 2020 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pwnage101 pwnage101 pwnage101 approved these changes

@brianhw brianhw Awaiting requested review from brianhw

@estute estute Awaiting requested review from estute

@HassanJaveed84 HassanJaveed84 Awaiting requested review from HassanJaveed84

@hunytalk hunytalk Awaiting requested review from hunytalk

@macdiesel macdiesel Awaiting requested review from macdiesel

+1 more reviewer

@bmedx bmedx bmedx approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@doctoryes @edx-status-bot @edx-pipeline-bot @pwnage101 @bmedx