Review Request 127910: kdesrc-build: improve error messages by showing the right filename

Michael Pyne mpyne at kde.org
Sun May 15 19:20:06 UTC 2016


-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20160515/d59ffeb1/attachment.html>


More information about the Kde-buildsystem mailing list