[Intel-gfx] [maintainer-tools PATCH 1/2] dim: Accept .mbox and .patch files as apply-branch optional argument.

[Daniel Vetter]

On Thu, Aug 17, 2017 at 10:30:02AM +0300, Jani Nikula wrote:
> On Wed, 16 Aug 2017, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> > Instead of forcing users to cat .patch or .mbox let's accept them
> > as optional argument for dim apply-branches.
> 
> Well, that's a useless use of cat anyway. You could do
> 
> $ dim apply-branch branch < patch.mbox
> 
> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > ---
> >  dim     | 10 +++++++++-
> >  dim.rst |  2 +-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/dim b/dim
> > index 11aa675cc3bc..e98d23b24ec0 100755
> > --- a/dim
> > +++ b/dim
> > @@ -771,7 +771,15 @@ function dim_apply_branch
> >  	assert_branch $branch
> >  	assert_repo_clean
> >  
> > -	cat > $file
> > +	case $1 in
> > +		*".patch" | *".mbox")
> > +			cat $1 > $file
> > +			shift
> > +			;;
> > +		*)
> > +			cat > $file
> > +			;;
> > +	esac
> 
> This would really be a surprising interface, argument parsing based on
> file suffixes. I don't approve.
> 
> You'll need to make this handle options before the branch argument,
> something like:
> 
> Usage: dim apply-branch [apply-branch options] branch [--] [git options]
> 
> Is stdin redirection really such a bad thing?

+1 on that. If I pick it from patchwork or an mbox, I just < patch.mbox or
curl patchwork.url | dim apply. I don't see the value in baking that into
the script ...

What we could do (and we have a jira for that already) is to check
patchwork for the updated .mbox with all the r-b/t-b/a-b tags auto-added.
That would greatly improve at least my workflow.

And I also agree with Jani on keeping the dim design clean. We can add
stuff where it makes sense, and where dim really can add value (with
additional safety checks and convenience features).

One thing we maybe could be doing is a more fancy aliasing feature, along
the lines of git aliases, where you can do whatever you want. Then you
could do a dim apply-curl alias which resolves to curl $arg | dim apply.
Not sure how to implement that though ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch