[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [59848] trunk/blender/source/blender: Code cleanup: use boolean instead of int for colormanagement

[Sergey Sharybin]

Hi,

I totally see your point, and agree it's not fun at all to svn merge. Ok, i
night have been violated first rule of code style but, but you might
consider this not a cleanup, but just regular development getting rid of
real traps.

Sometimes it happens that (butflag & FLAG) is passed to a bool argument.
Which you never shall do. It need to be (butflag & FLAG) != 0. Was getting
rid of those. This might easily run into situation when you pass (butflag &
FLAG) via public API which uses int but then calls internal functions which
are bool.. Really asking for huge issues.

Wanted to be 100% sure bitflag is never gets trunkated by the cast to bool
(in CM domain). This kind of issues are just real PITA to notice and
troubleshoot.

Maybe not an excuse to introduce pain for merge, but imo it's still better
than having traps in my code (which i was running into in own WIP patches
already).

As for general rule: we do not do cleanup which could wait for after GSoC.
But if re-arranging code removes potential bugs i would say it's more
"general development", not "cleanup, nu functional changes" and should be
fine for commits (IMO).



On Mon, Sep 16, 2013 at 2:48 AM, Dalai Felinto <dfelinto at gmail.com> wrote:

> Hi Sergey,
>
> Thanks for the lengthy reply, please bear with me:
>
> First, do you mind educating me on the practical differences on using true
> over TRUE? I've never heard of an unsafe "int<->bool cast. (pure lack of
> knowledge on my end here).
>
> And if there is a relevant difference, can we decide on one of them (I
> thought we had already, actually) to use across the code?
>
> Second, the first rule of code style I've been using (and seem to be
> reinforced by  Blender's guideline [1]) is to be consistent within the
> file. Thus it strikes me as a strange procedure to do code cleanup in parts
> of a file, but not in all of it. (but again, I may be ignorant on the
> implications of using TRUE or true in different parts of the code). I can
> also guarantee that a new coder looking at working in those files will be
> just as confused.
>
> Last and not least, it's not 'my branch', and far from me to propose
> halting trunk development. But we are close to the conclusion of the GSoC,
> and there will be a few more branches that will need to keep up with trunk
> and may face the same situation.
>
> All that said, I don't mind to even agree to disagree here.
> I just wanted to voice my 2 cents.
>
> Cheers,
> Dalai
>
> [1] "When making changes, conform to the style and conventions of the
> surrounding code" - http://wiki.blender.org/index.php/Dev:Doc/CodeStyle
>
> --
> blendernetwork.org/dalai-felinto
> www.dalaifelinto.com
>
> [re-sending as a clean reply because it was too big and held for
> moderation]
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers
>



-- 
With best regards, Sergey Sharybin