kicad-developers team mailing list archive

[Jacobo Aragunde Pérez <jaragunde@xxxxxxxxxx>]

El 14/01/13 14:00, Miguel Angel Ajo Pelayo escribió:
> Hi Jacobo, thanks for the patch!, 
> 
>     Doesn't look bad for me, but please check:
> 

Hello, thanks for the review :)

>    * You removed the WINDOWS special case, does it work in Windows now
> without removing
> the "aFlags" ? 
> 
> -bool ProcessExecute( const wxString& aCommandLine, int aFlags )
> +int ProcessExecute( const wxString& aCommandLine, int aFlags, wxProcess
> *callback )
>  {
> -#ifdef __WINDOWS__
> -    int        pid = wxExecute( aCommandLine );
> -    return pid ? true : false;
> -#else
> -    wxProcess* process = wxProcess::Open( aCommandLine, aFlags );
> -    return (process != NULL) ? true : false;
> -#endif
> +    return wxExecute( aCommandLine, aFlags, callback );
>  }
>  

So the purpose of the #ifdef was removing the flags from the call, I
see... I'll try to test in a Windows box, in any case the setup will be
useful in the future.

> 
>    * Line 280 of patch: +    if( pid > 0 ) {    <-- check coding style
> policy, opening brace must be at the next line.
>    
>    I checked it manually, all the other coding style looks right to my eye.
> 

Oops! Too many years using that coding style :D


-- 
Jacobo Aragunde
Software Engineer at Igalia