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
Revert "Revert "Remove deprecated :text option to render"" #37580
Revert "Revert "Remove deprecated :text option to render"" #37580
Conversation
@@ -142,7 +142,7 @@ def pd_progress | |||
) | |||
render( | |||
layout: 'application', | |||
text: "PD progress data not found for #{sanitized_script_name}.", | |||
plain: "PD progress data not found for #{sanitized_script_name}.", |
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.
These two seem worth verifying before merging since they also specify a layout.
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.
oh wow, nice catch; I literally looked right at these and didn't see a layout
🙃
… also added tests
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.
thank you for adding tests!
Reverts #37571, restoring #37509
It turns out I was slightly wrong about my understanding of the deprecation.
render text
used to be able to either render just a raw text response OR render text into an html layout, with the use of the layout keyword. This behavior was confusing, sorender text
was broken up intorender plain
andrender html
; in my upgrade, I naively updated all uses ofrender text
torender plain
, breaking the single use of it that should have beenrender html
.That update has now been fixed, and a test case has been added that would have caught the regression.