[Bf-funboard] UI talk + session at bconf

Campbell Barton ideasman42 at gmail.com
Fri Sep 27 13:45:47 CEST 2013


On Fri, Sep 27, 2013 at 3:20 AM, david j <davidj at gmail.com> wrote:
> On Sep 26, 2013 6:44 AM, "Brecht Van Lommel" <brechtvanlommel at pandora.be>
> wrote:
>> Regarding that spline handle change, it seems like a big change that
>> would have an impact on animation workflow, and the one reply you got
>> from an experienced animator is that they don't consider it a bug.
>
> I made two different patches, one with the behavior change and one without.
> You are talking about the one with a behavior change. Below is the one I
> was referring to which does *not* change behavior...
>
> https://projects.blender.org/tracker/?func=detail&aid=36048&group_id=9&atid=127
>
> I don't want to derail this into a discussion about my tiny patches, but I
> do think it's an interesting micro-view into the process proposals I'm
> talking about.
>
> Even that tiny patch above, which changes no behavior, has a "UI Change".
> While talking with artists about my proposed behavior change, they asked if
> I could add visual feedback for rotation constrained handles. I was already
> knee deep in that code, so it was trivial to add *something* (I draw the
> handle as a "T" if it is rotation constrained) A committer accepting the
> patch is put in the odd position of also approving that UI change.
>
> If there was a separate UI-approval process, I would have created that
> ticket, with screenshots, had the artists weigh in on it, possibly with
> better visual ideas, someone would have approved it, and my patch would
> contain a link to the UI approval.
>
> Returning to the contentious proposed behavior change, it's true I received
> conflicting feedback about it. However, what I did not receive is any well
> considered choice about which was is better. Saying it is "not a bug", is
> not the same thing as "this is the right path for the future of bender".
> For example, in this case, there are reasons to reconsider how this works.
> When discussing on IRC, one person who "likes it the way it is", also said
> they would like the full vocabulary of spline handle manipulations from
> other tools, including mirroring --- which do handle-sets like my proposed
> behavior change.
>
> Let's try to avoid starting a discussion here about the handle-set change,
> which isn't my aim. We can do that elsewhere. My point is, UI design
> decisions are not made by committee. A bunch of voices saying "don't change
> it" and other voices saying "do change it" does not lead to a well informed
> decision.
>
> This is what I'd like to see out of a structured UI approval process. Even
> if the person with authority over that decision disagrees is not convinced,
> which is very possible, at least decisions would be getting made allowing
> possible progress. From my limited view, I'm not the only one who feels
> progress can get stalled merely for lack of any way to get a decision made.
>
>> Well, the whole point of the review is that it's not possible to trust
>
>> a developer not experienced in some area to get it right.
>
> I understand that is the BF reason for code reviews, but this is not how
> all projects view code reviews.
>
> IMO, code reviews should not be about paranoia and distrust, but about
> raising the quality bar because two eyes are better than one. Having code
> "seen" by someone else helps enforce coding standards and generate better
> ideas. This doesn't require them to deeply understand the code or issues,
> it just requires them to take a look, and ask a question or two.
>
> As long as they can trust a UI approval process took care of making sure
> the UI changes are approved, they understand enough to trivially
> QA/validate the behavior of the code, and the alpha/beta/RC release process
> will catch bugs which slip through, this level of distrust in code-reviews
> is not necessary.
>
> The irony, viewed through my micro-patch above, is that the function my
> patch rewrites would never pass any code-review I've ever been a part of.
>
> Cleanly separating the UI-approval process from code-review would have
> helped clarify the path forward for both patches.
>
> Changing the mentality about code-reviews in blender, now that the release
> process is improved, will help more efficiently leverage this work from
> external developers, and also encourage them to make more contributions by
> helping them feel their work is appreciated instead of needless sitting on
> the vine.

I've been there - Where there is crappy code which little sense and
nobody is actively improving.
(We mostly just maintain curve code, add features every so often).

But its also hard for us to accept changes from someone who isn't an
expert in the area.

You make some changes that are subjective and we don't have an experts
who can properly give you feedback - so the patch doesn't get
reviewed.

Its a problem. But if you are interested in becoming involved in
Blender development (as opposed to just fixing this one issue),
you could pick tasks that we have developers to review, or obvious
improvements that aren't so subjective.

For example - if you submit a change to cycles/bmesh/uv-editing - we
can certainly review these and give you good feedback.

Another example, recently a developer worked on curve support for
deleting and splitting off curves by segments.
Since this is an obvious improvement I reviewed and committed.


Basically as a newer blender-developer you have to pick your battles,
and it helps if you choose areas where developers can give you good
feedback.

If you work in an area for some time you can become a maintainer, and
be responsible when users complain things don't work properly :)
but that cant be rushed - so Id suggest to become involved with any
area (curves for eg) bit by bit.

Nobody will complain if you add freehand drawing for eg, or fix
extrude behavior for cyclic splines...
You could plan out your tasks a bit check with developers who would
review, what they think before you work on them.

-- 
- Campbell


More information about the Bf-funboard mailing list