Review Request 124613: kdesrc-build: improve message about why a full refresh is needed

[Michael Pyne]

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



modules/ksb/BuildSystem.pm (line 73)
<https://git.reviewboard.kde.org/r/124613/#comment57774>

    This file is only created by the user. This message belongs with "$confFileKey"'s condition below.
    
    The message here should be "the build directory was manually marked out-of-date" or something to that effect.



modules/ksb/BuildSystem.pm (line 79)
<https://git.reviewboard.kde.org/r/124613/#comment57775>

    Would recommend a message like '$count previous attempts to build this module have failed' (or even just remove the count).



modules/ksb/BuildSystem.pm (line 82)
<https://git.reviewboard.kde.org/r/124613/#comment57776>

    *this* is the one that should be "the last configure failed". ;)


One other thing, the "l10nSystem" class is a source updater and a build system combined, and has a 'needsRefreshed' sub that would also need updated. Something like this diff should work:

    diff --git a/modules/ksb/l10nSystem.pm b/modules/ksb/l10nSystem.pm
    index e5756f8..0101824 100644
    --- a/modules/ksb/l10nSystem.pm
    +++ b/modules/ksb/l10nSystem.pm
    @@ -24,7 +24,9 @@ sub new
         # TODO: Support different localization branches?
     
         $module->setOption('module-base-path', 'trunk/l10n-kde4');
    -    return bless { module => $module, needsRefreshed => 1 }, $class;
    +
    +    my $refreshMessage = "couldn't verify the source is unchanged";
    +    return bless { module => $module, needsRefreshed => $refreshMessage }, $class;
     }
     
     sub module
    @@ -75,7 +77,7 @@ sub updateInternal
             $self->check_module_validity();
             my $count = $self->update_module_path(@dirs);
     
    -        $self->{needsRefreshed} = 0 if $count == 0;
    +        $self->{needsRefreshed} = '' if $count == 0;
             return $count;
         }
         else {
    @@ -103,7 +105,7 @@ sub needsRefreshed
     {
         my $self = shift;
     
    -    # Should be 1 except if no update happened.
    +    # Should be a 'reason' string except if no update happened.
         return $self->{needsRefreshed};
     }

With those taken care of you have a +1 from me.

- Michael Pyne


On Aug. 4, 2015, 9:57 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124613/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2015, 9:57 a.m.)
> 
> 
> Review request for Build System and Michael Pyne.
> 
> 
> Repository: kdesrc-build
> 
> 
> Description
> -------
> 
> i.e. be more precise than "meets other building criteria"
> 
> 
> Diffs
> -----
> 
>   modules/ksb/BuildSystem.pm 24963040d77dc93f8f86a2f19755c617713bbd8a 
>   modules/ksb/IPC.pm 0e55c5fe5c2a8771f6b9b159a684bdcc6dda52a7 
>   modules/ksb/Module.pm a7a7fbcc5e30cbb4c52979d87407a3250b80f2b2 
> 
> Diff: https://git.reviewboard.kde.org/r/124613/diff/
> 
> 
> Testing
> -------
> 
> Example:
> 
> $ kdesrc-build --refresh plasma-nm
> 
> Building plasma-nm from kf5-network-management (1/1)
>         Updating plasma-nm (to branch master)
>         No source update, but the option refresh-build was set
>         Source update complete for plasma-nm: no files affected
>         Preparing build system for plasma-nm.
> ...
> 
> 
> Thanks,
> 
> David Faure
> 
>

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