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

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 16 Aug 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Instead of having to manually download mbox from patchwork
> let's make dim to do it directly.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  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?

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

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

---

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.


BR,
Jani.


>  		*".patch" | *".mbox")
>  			cat $1 > $file
>  			shift

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx