-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Proxito: remove old implementation #10660
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a quick look at this and it looks good. I didn't jump pretty deep into reviewing this code because it's mostly removal of an old implementation. I think we need to fix the tests and we are ready to go 🚀
migrations.RemoveField( | ||
model_name="project", | ||
name="urlconf", | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't deploy break if we remove this field? I assume the current production code is not using this field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to run this migration after we have deployed the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK! Make sure to make this pretty clear on the release project. We usually run migrations before deploying new code.
Mostly removing old code, there is only one small custom addition
readthedocs.org/readthedocs/proxito/views/serve.py
Lines 144 to 146 in 6595bda
Django doesn't capture the first slash, previously we were getting the path from request.path_info, that includes the slash! When a request is done to
/
the path is set to "" (empty), our code relies on that being/
./en/
or/zh/
as path prefix with single version #8399How to deploy this change
projects 0105
migration!!!projects 0105
migration