[Bf-committers] Code cleanup rules

Thomas Dinges blender at dingto.org
Thu May 1 13:17:23 CEST 2014


Hi Sergey,
for the record I was against this too. I fixed it, by adding regular 
includes back. 
https://developer.blender.org/rBa47a4ef82f37428d391cc14a30fa611d6714e71d
Campbell reverted that and commited the platform based include version.
Later on I found 2 compile errors with vc2008 and he suggested to use 
the ifdef that checks for MSVC and the version.

In General, I ask everyone to stop cleaning up vc2008 stuff now! We will 
drop vc2008 in a few weeks probably (after 2.71 is out), so until then, 
just leave it as is.

Am 01.05.2014 08:20, schrieb Sergey Sharybin:
> Aha, the thread has been already started here. Would summarize my two
> messages here.
>
> Doing an if-defed inclusion of own files based not only the platform but
> also based on compiler and it's version is just STUPID. I do want to hear a
> weighful point on this, and not just "to reduce amount of re-compile time
> after the cleanup".
>
> The point is, if some .c|.cc|.cpp file needs some function it IS to include
> a header file here this function is declared and NEVER rely on indirect
> inclusion. I simply don't believe in cases when function is only used on
> windoze from this snippet:
>
> #ifdef WIN32
> #  include "BLI_string.h"  /* BLI_strcasecmp */
> #endif
>
> So please do stick to a rule "include all you're using and no more than
> this in the implementation file". With all this changes you've just
> increased "WTF IS GOING ON??" factor when reading the affected files.
>
> Further, with all this compiler-specific includes you've made it so only
> compilers you're compiled will work. Did you test armel architecture? Did
> you test hurd kernel? Did you test BSD after all? I do believe with all
> this "cleanup" some of the platforms might easily became broken.
>
> And one last thing, please DO NOT do such a cleanup silently from .au while
> maintainers of all other platforms are sleeping because of timezone lag.
> Such things are to be discussed first and i would tell you my negative
> feeling about ifdef-ed includes on the early stages.
>
>
>
> On Thu, May 1, 2014 at 3:38 AM, Chad Fraleigh <chadf at triularity.org> wrote:
>
>> Just a style question..
>>
>> Wouldn't it be better for the BLI_*.h files to do the WIN32 (or other)
>> ifdefs and otherwise be used platform generic, rather than make each C file
>> that uses them do the ifdef?
>>
>> -Chad
>>
>>
>> On Wed, Apr 30, 2014 at 2:27 PM, Thomas Dinges <blender at dingto.org> wrote:
>>
>>> Hi,
>>> Next time you do such massive cleanup, you can also just spend 10mins of
>>> your time and boot your Windows installation before commit.
>>> I spend 30min of my time now to fix the issues, and then you revert that
>>> 5minutes afterwards. Very nice.
>>>
>>> I clearly vote for more strict rules in regards to code cleanups that
>>> affect all areas/and OS. This madness has to stop.
>>>



More information about the Bf-committers mailing list