-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
escape special characters to deserialize changes for column renaming operation #7448
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
tfmorris
left a comment
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.
Thanks for tackling this. Because it addresses a critical data reliability area, we want to make sure it has excellent test coverage. Could you please add some tests?
Also, the original issue mentions issues with deserializing existing corrupted projects, which this doesn't seem to address.
|
p.s. because this is likely a much bigger task than you anticipated, you are welcome to switch to a more manageable sized task, if that's your preference. |
|
Thanks for additional updates. I apologize for not noticing before that this PR was created from your master branch, but that's going to be very susceptible to inadvertent changes. Could you please move these to a feature branch and open a new PR based on that? You can do that by something along the lines of to create the new branch and then commit your changes to that, perhaps by using I will start my review in parallel with that process. |
|
#7461 |
Fixes #7341
0922.mp4