-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127910/#review95490
-----------------------------------------------------------


Fix it, then Ship it!




Looks good aside from one remaining issue!


modules/ksb/RecursiveFH.pm (line 45)
<https://git.reviewboard.kde.org/r/127910/#comment64729>

    `$result` here is the old filehandle (so it can be closed) so this boolean 
won't work.
    
    If you wanted to be fancy you could use comma operator perhaps, but it's 
probably best to just put the filename stack update on a separate line.
    
    Alternately we could close the fh here and free up the calling code from 
doing it, and then return nothing... might be clearer that way.


- Michael Pyne


On May 15, 2016, 12:59 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127910/
> -----------------------------------------------------------
> 
> (Updated May 15, 2016, 12:59 p.m.)
> 
> 
> Review request for Build System and Michael Pyne.
> 
> 
> Repository: kdesrc-build
> 
> 
> Description
> -------
> 
> Before:
> "Don't use module libaccounts-qt on line 20 of /path/kdesrc-buildrc, use 
> options libaccounts-qt"
>  but line 20 is unrelated, some global option.
> 
> After:
> "Don't use module libaccounts-qt on line 20 of 
> /path/extragear/utils/kdesrc-build/kf5-workspace-build-include, use options 
> libaccounts-qt"
> 
> 
> Diffs
> -----
> 
>   modules/ksb/Application.pm 5dbd224c7ba06242b53cd77cfa0764c28da76579 
>   modules/ksb/RecursiveFH.pm 6892320bb5e8f8c1e4979ef137bb975775940908 
> 
> Diff: https://git.reviewboard.kde.org/r/127910/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Faure
> 
>

_______________________________________________
Kde-buildsystem mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-buildsystem

Reply via email to