Re: [maintainer-tools PATCH 2/2] dim: Accept patchwork URLs as apply-branch optional argument.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 17, 2017 at 12:37 AM, Jani Nikula <jani.nikula@xxxxxxxxx> wrote:
> On Wed, 16 Aug 2017, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
>> Instead of having to manually download mbox from patchwork
>> let's make dim to do it directly.
>>
>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
>> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>> ---
>>  dim | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/dim b/dim
>> index e98d23b24ec0..73b48da7f436 100755
>> --- a/dim
>> +++ b/dim
>> @@ -756,6 +756,16 @@ function dim_push
>>       dim_push_branch $(git_current_branch) "$@"
>>  }
>>
>> +function download_mbox
>> +{
>> +     wget -q --spider ${1}
>
> What's the benefit of doing this first?

it check if links exist before trying to download anything.
at least it returns fast when link is not valid....

>
>> +     if [ $? -ne "0" ]; then
>> +         echoerr "URL ${1} not found."
>> +         exit 1
>
> Always just return 1 on errors, and set -e will handle the abort. This
> way the caller can decide to use foo || true to ignore the error.

oh of course!

>
>> +     fi
>> +     wget -q ${1} -O $2
>> +}
>> +
>>  # ensure we're on branch $1, and apply patches. the rest of the arguments are
>>  # passed to git am.
>>  dim_alias_ab=apply-branch
>> @@ -772,6 +782,14 @@ function dim_apply_branch
>>       assert_repo_clean
>>
>>       case $1 in
>> +             *"patchwork.freedesktop.org"*"mbox")
>> +                     download_mbox $1 $file
>> +                     shift
>> +                     ;;
>> +             *"patchwork.freedesktop.org"*)
>> +                     download_mbox $1/mbox $file
>> +                     shift
>> +                     ;;
>
> Really, the interface gets worse and worse here!

actually was the other way around...
I made this bad one and decided to extend to files :P

>
> ---
>
> I may sound like a pedant looking after style and consistency in a shell
> script, but this one has grown beyond the size where we can just ignore
> its maintenance. I've put in quite a bit of effort since the user base
> went from 1 to 2 to keep it together.

you are absolutely right. it is not just a shell script... it is our
tool and we should care!

>
>
> BR,
> Jani.
>
>
>>               *".patch" | *".mbox")
>>                       cat $1 > $file
>>                       shift
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux