This repository has been archived by the owner on Sep 20, 2024. It is now read-only.
General: missing comment on standalone and tray publisher #4303
+5
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Check for context must be preserved until old Pyblish is completely eradicated as comment could be filled after collection phase, therefore not caught by collector.
Printing what is actually missing is more helpful than only full data.
kalisp
added
type: bug
iLLiCiTiT
reviewed
Jan 10, 2023
| @@ -45,7 +45,8 @@ def process(self, instance): | |||
| host_name = context.data["hostName"] | |||
| app_name = context.data["appName"] | |||
| app_label = context.data["appLabel"] | |||
| comment = instance.data["comment"] | |||
| # context comment is fallback until old Pyblish is removed | |||
| comment = instance.data["comment"] or context.data.get("comment") | |||
There was a problem hiding this comment.
This is duplicate of what happens here https://github.com/ynput/OpenPype/blob/develop/openpype/plugins/publish/collect_comment.py#L99
The "comment" should be always filled. Maybe only if instance is created after the collector?
Pyblish allows modifying comment after collect phase, eg. collector wouldn't collect it. Should be pushed back to Collect phase after Pyblish is eradicated.
Availability of comment on instance has been resolved by bumping up order of CollectComment.
|
Pushed collector to Extract phase because of Pyblish. Marked with TODO, should be refactored when Pyblish is gone. |
iLLiCiTiT
approved these changes
Jan 11, 2023
|
there is just a slight inconsistency in terminology. There are three terms used and it may be a bit confusing. We are writing a |
kalisp
deleted the
bugfix/OP-4679_Issue-Comment-on-Standalone-and-tray-Publisher
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Brief description
Comment might have not been collected when inserted only after collection phase which resulted in not sending comment to Ftrack.
Additional info
Clean-up of comment and using only comments on instances is good step, but as far as we are still using old Pyblish, context comment should be kept as a fallback (comment might be filled after collection phase, in this case it is present only on context level. Eg. artists knows/sees that it is filled, but integrator prints it is not there. Another approach would be to disable comments field after publish is started, but I didn't want to mess with Pyblish.)
Comment field in Publisher is controlled by
project_settings/global/publish/collect_comment_per_instance. To show comment field this should be set:Testing notes:
Integrate Ftrack noteand Ftrack for presence of commentintentandcomment, skippingintentis quite easy in SP, in Publisher field for it is missing completely, imho)