Skip to content
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

Conversation

hannahmkrasnick
Copy link
Contributor

@hannahmkrasnick hannahmkrasnick commented Sep 30, 2024

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

Copy link

linux-foundation-easycla bot commented Sep 30, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@carolalynn carolalynn added the devdays24 Label for PRs for Dev Days 2024. label Sep 30, 2024
Copy link
Collaborator

@doug-walker doug-walker left a 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.

src/apps/ociodisplay/main.cpp Outdated Show resolved Hide resolved
@doug-walker
Copy link
Collaborator

@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?

@hannahmkrasnick
Copy link
Contributor Author

@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!

@hannahmkrasnick hannahmkrasnick force-pushed the bug-fix/fix_ociodisplay_view_menu branch from fe74f08 to f886de0 Compare November 28, 2024 21:04
Copy link
Collaborator

@doug-walker doug-walker left a 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.

@doug-walker doug-walker merged commit 5c2fa57 into AcademySoftwareFoundation:main Dec 7, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devdays24 Label for PRs for Dev Days 2024.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants