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

DotNodeGadget : Use ContextTracker to evaluate custom label #5994

Merged
merged 2 commits into from
Aug 7, 2024

Conversation

johnhaddon
Copy link
Member

This also fixes a bug where in the past we didn't update context-sensitive labels when the global context changed.

@johnhaddon johnhaddon self-assigned this Aug 6, 2024
@danieldresser-ie
Copy link
Contributor

LGTM. The only thing that gave me pause was confirming that it's OK to pass a null context to Context::Scope - after looking at it, pretty sure it is the convention that this must be valid, and doesn't do anything to the stack. Probably doesn't need documenting here, but it might be worth documenting in the header for Context, which currently says: "/// Constructing the Scope pushes the current context." and "/// It is the caller's responsibility to guarantee that context outlives the EditableScope." - neither of which really acknowledges that it's OK for the context to be null.

This also fixes a bug where in the past we didn't update context-sensitive labels when the global context changed.
@johnhaddon
Copy link
Member Author

Added suggested comment in Context.h, and merging...

@johnhaddon johnhaddon merged commit 9599500 into GafferHQ:main Aug 7, 2024
6 checks passed
@johnhaddon johnhaddon deleted the dotContextTracker branch August 7, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants