chore: Use codespell to fix typos in code.#4797
Conversation
Second part of replacement of ietf-tools#4651 @rjsparks, I probably need to revert some things here, and I also still need to add that new migration - how do I do that?
Codecov Report
@@ Coverage Diff @@
## main #4797 +/- ##
==========================================
- Coverage 88.46% 88.45% -0.01%
==========================================
Files 296 296
Lines 39755 39755
==========================================
- Hits 35169 35166 -3
- Misses 4586 4589 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
After you change the model, run |
|
I just tried |
|
Because you changed the names in the already existing migrations. Changing already applied migrations is never the right thing to do (unless you plan to run the reverse and forward again on the production database). Since you changed ietf/mailinglists/migrations/0001_initial.py, Django thinks that the class has always been named Allowlisted, and doesn't know that it needs to make changes to the database. |
|
Thanks! I will try to fix next week. |
|
To be clear, I think it's best to just revert all the existing migrations to what's in main. |
| email.save() | ||
| PersonEvent.objects.create(person=email.person, type='email_address_deactivated', | ||
| desc="Deactivated the email addres <%s>. Reason: %s" % (email.address, options['reason']) ) | ||
| desc="Deactivated the email address <%s>. Reason: %s" % (email.address, options['reason']) ) |
There was a problem hiding this comment.
This will make PersonEvent objects created in the future correct, but will not change any that are already in the database.
>>> PersonEvent.objects.filter(desc__contains="addres ").count()
5640
To change those, we would need a data migration. (This would have a forward and a reverse, but the reverse should do nothing (just pass) as it doesn't make sense to go back to misspelled descriptions).
There was a problem hiding this comment.
How would I add one?
There was a problem hiding this comment.
Several steps.
- `ietf/manage.py makemigrations --empty --name=personevent_spelling person
- Add a forward and reverse function to the empty migration it creates and wire them into
migrations.RunPython(search fordef forward(to find lots of examples. - update the dependencies list to the tip of any apps from which you end up pulling in the model. (You'll at least pull in PersonEvent from Person, using
apps.get_model, not an import, but you'll get the right person dependency automatically given that you made the migration on the person app. Note also that what you get is a facade with a working object manager, not the actual class - member functions of the class will not be available). - the reverse should just
pass. For the forward, you'll want to figure out how to iterate over the objects that have the spelling errors and update them as efficiently as you can (since however long this runs is downtime as the release it contains is deployed).
More to read at https://docs.djangoproject.com/en/2.2/topics/migrations/
There was a problem hiding this comment.
I think this is beyond my paygrade...
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
Missed renaming a method after fixing the same word in its code. Should fix that while in the area.
A couple other minor suggestions inline that can be taken or left as you prefer.
3 participants
Second part of replacement of #4651
@rjsparks, I probably need to revert some things here, and I also still need to add that new migration - how do I do that?