search

pyblish / pyblish-base

Pyblish base library - see https://github.com/pyblish/pyblish for details.
Other
127 stars 59 forks source link

"publish" -> "active" #307

Open mottosso opened 7 years ago

mottosso commented 7 years ago

Goal

Clarify meaning and use of publish data member in instances.

instance.data["publish"] = False

Motivation

Adding the boolean member publish to an instance indicates whether or not it should be published. In the GUI, this member is represented by toggles.

Initially, the word was chosen because an instance was created and either published, or not published. So "publish" seemed the most logical name. But in this case, your second collector doesn't necessarily publish the instance and so the logic breaks.

But when producing an instance across two or more collectors, "publish" no longer makes logical sense. For example, one collector might create it, another might add additional members to it.

Implementation

Rename member to active, like it is referred to in plug-ins.

instance.data["active"] = True

This would not only make more logical sense, but also unify terminology with plug-ins.

BigRoy commented 7 years ago

Actually; @mottosso how does this compare with https://github.com/pyblish/pyblish-base/blob/17e483a14cecba136f59bf262e30d1f6e9478975/pyblish/plugin.py#L201 ?

I remember we already had a use for active somewhere? Did you remember what that was? Was that disabling it completely? And is it getting more confusing if we also rename this key?

mottosso commented 7 years ago

Looks like it's there, but not used. I can't find a use of it anywhere. It should be though, so long as there's backwards compatibility with publish. I don't think it's confusing personally. Plug-ins do this, and actions too. Did you think it was, or how come you ask?

BigRoy commented 7 years ago

I remember it being implemented where active would force a plug-in to be unused completely, whereas publish was one that artists could toggle afterwards. Almost to a point where active was unable to be altered at runtime by the user.

Note that active is used in util: https://github.com/pyblish/pyblish-base/blob/477f540981433867aa219cc46cd0fa5890ecf17c/pyblish/util.py#L49 In the logic: https://github.com/pyblish/pyblish-base/blob/477f540981433867aa219cc46cd0fa5890ecf17c/pyblish/logic.py#L364

And in tests: https://github.com/pyblish/pyblish-base/blob/477f540981433867aa219cc46cd0fa5890ecf17c/tests/test_plugin.py#L731

Since both are currently implemented I'm expecting that renaming one will add to the confusion.

BigRoy commented 7 years ago

Ah, wait. Nevermind. So I think I must have been mistaking. Basically you're correct. The idea is to also use the terminology of instance.data["active"] just like it was already being used for plug-ins.

Totally correct!

So in that way both plug-ins and instances will use active to toggle that state. Just like you mentioned in the original issue:

Rename member to active, like it is referred to in plug-ins.

mottosso commented 7 years ago

Basically you're correct

The best form of correct!