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

oneoff script to remove lesson descriptions from scripts.en.yml #46216

Merged
merged 4 commits into from
May 6, 2022

Conversation

davidsbailey
Copy link
Member

@davidsbailey davidsbailey commented May 6, 2022

Step 6a for PLAT-1528. Depends on #46188.

This PR adds a oneoff script which will be run on levelbuilder to remove unneeded lesson fields from scripts.en.yml, specifically: key, description_student and description_teacher. I am doing it this way rather than just making the change to scripts.en.yml in my PR in hopes of avoiding merge conflicts.

before

$ wc -l dashboard/config/locales/scripts.en.yml
   37252 dashboard/config/locales/scripts.en.yml

after

$ wc -l dashboard/config/locales/scripts.en.yml
   25578 dashboard/config/locales/scripts.en.yml

Testing story

take a look at e4684c1 to see the results of running this script on my local machine. unfortunately, because scripts.en.yml is so big, you will have to look at the diff on your local machine. after diffing this locally, I inspected it to make sure that only the fields specified above were being removed.

@davidsbailey davidsbailey changed the title Remove lesson description yml Remove lesson descriptions from scripts.en.yml May 6, 2022
@davidsbailey davidsbailey changed the title Remove lesson descriptions from scripts.en.yml oneoff script to remove lesson descriptions from scripts.en.yml May 6, 2022
@davidsbailey davidsbailey requested a review from a team May 6, 2022 15:39
@davidsbailey davidsbailey marked this pull request as ready for review May 6, 2022 15:39
@dmcavoy
Copy link
Contributor

dmcavoy commented May 6, 2022

Based on our conversation here: #46188 (comment), my understanding is there are still scripts in production relying on these strings.

@davidsbailey
Copy link
Member Author

Not sure if this got resolved during stand up, but my understanding is that we are no longer using these strings as of #46143 . lmk if you want to jump on a call to discuss!

@dmcavoy dmcavoy closed this May 6, 2022
@dmcavoy dmcavoy reopened this May 6, 2022

require_relative '../../dashboard/config/environment'

units_yml = File.expand_path("#{Rails.root}/config/locales/scripts.en.yml")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think its blocking but wondering if it would be safer to make sure we are on levelbuilder or local when running this script

Copy link
Member Author

Choose a reason for hiding this comment

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

good call -- done!

Copy link
Member Author

Choose a reason for hiding this comment

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

I reran it and confirmed that it still works in local development.

@dmcavoy
Copy link
Contributor

dmcavoy commented May 6, 2022

#46188 (comment)

Not sure if this got resolved during stand up, but my understanding is that we are no longer using these strings as of #46143 . lmk if you want to jump on a call to discuss!

A misunderstanding on my part. Thanks for clarifying

@davidsbailey davidsbailey merged commit a807583 into staging May 6, 2022
@davidsbailey davidsbailey deleted the remove-lesson-description-yml branch May 6, 2022 20:23
@davidsbailey
Copy link
Member Author

for deployment strategy, I am thinking:

  1. wait for this to be deployed to levelbuilder
  2. wait for 1-2 days of LB content scoops. confirm that none of them touch lines containing description_student or description_teacher
  3. run the script on levelbuilder. commit the resulting changes to scripts.en.yml in its own commit
  4. in the unlikely event that this causes a production problem, you could try reverting that one commit (or, due to merge conflicts, more likely resetting to just before that one commit).

I can just plan to do this when I am back, but if there is reason to move forward on this sooner, it could be done in my absence.

@davidsbailey
Copy link
Member Author

wait for 1-2 days of LB content scoops. confirm that none of them touch lines containing description_student or description_teacher

just noting that this has been confirmed for the 4 most recent content scoops, which are all of the scoops since #46188 reached levelbuilder.

@davidsbailey
Copy link
Member Author

run the script on levelbuilder. commit the resulting changes to scripts.en.yml in its own commit

done in 7d35bcf

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.

None yet

2 participants