-
Notifications
You must be signed in to change notification settings - Fork 460
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
[2057] - Fix views/display submenus in OCIODisplay app #2068
[2057] - Fix views/display submenus in OCIODisplay app #2068
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.
Thanks for this contribution Hannah!
Apologies for the delayed review, things have been very busy. Just one minor request below.
Minor issue, but I'm noticing a large number of white space changes. Not sure if that was you or you have an auto-formatter turned on. Although the changes are not bad, we generally don't like people to reformat white space in areas of the code they are not modifying. It makes the PR more difficult to review and it makes the git blame history of the file harder to follow. I won't ask you to waste your time reverting the changes, but please keep it in mind for future contributions.
@hannahmkrasnick , we would like to merge this in time for the OCIO 2.4.1 release around Dec. 9. Have you been able to make progress on getting the CLA signed? |
Hey @doug-walker ! I've been added to the approval list, and I tried to update that here. Let me know if you need anything else from me! |
Signed-off-by: Hannah <[email protected]>
Signed-off-by: Hannah <[email protected]>
fe74f08
to
f886de0
Compare
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.
This is very helpful, Hannah! Thanks for getting the CLA signed, we will include this in OCIO 2.4.1.
This fixes the bug where the "Views" submenu (renamed from "Transform") doesn't update when a new "Display" is chosen (renamed submenu from "Device").
#2057