[PATCH 2/3] dim: Introduce dim_request_pull_tags

[Jani Nikula]

On Mon, 27 Aug 2018, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Fri, Aug 24, 2018 at 02:43:35PM -0700, Rodrigo Vivi wrote:
>> Besides drm-intel-next, sometimes it is used to re-use an
>> already existent tag that you just created outside
>> or in cases where mutt/smtp goof-up you don't want
>> to generate another identical tag with another date
>> or with "-1", "-2", etc
>> 
>> Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>> 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     | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>  dim.rst |  6 ++++++
>>  2 files changed, 51 insertions(+)
>> 
>> diff --git a/dim b/dim
>> index fec51f766e55..b4d7996a1072 100755
>> --- a/dim
>> +++ b/dim
>> @@ -1882,6 +1882,51 @@ function dim_tag_next
>>  	dim_tag_branch "drm-intel-next"
>>  }
>>  
>> +function check_tags # tags
>> +{
>> +	local tag
>> +
>> +	for tag in "$@";
>> +	do
>> +		if ! git rev-parse "$tag^{tag}" > /dev/null 2>&1; then
>> +			echoerr "Aborting: Tag ${tag} not found"
>> +			exit 1
>> +		else
>> +			echo $tag
>> +		fi
>> +	done
>> +}
>> +
>> +# dim_pull_request_tag branch upstream tag-list
>> +function dim_pull_request_tags
>> +{
>> +	local branch upstream repo req_file url_list git_url tags tag
>> +
>> +	branch=${1:?$usage}
>> +	shift
>> +	upstream=${1:?$usage}
>> +	shift
>> +	tags="$*"
>> +	repo=$(branch_to_repo $branch)
>> +	req_file=$(mktemp)
>> +
>> +	git_fetch_helper ${upstream%%/*}
>> +	echo "Using $upstream as the upstream"
>> +
>> +	# Sort with newest first
>> +	tags=$(check_tags $tags | sort -r)
>> +	tag=$(echo $tags | cut -d " " -f 1)
>> +
>> +	prep_pull_mail $req_file $tags
>> +
>> +	url_list=${drm_tip_repos[$repo]}
>> +	git_url=$(pick_protocol_url git $url_list)
>> +
>> +	git request-pull $upstream $git_url $tag >> $req_file
>> +	$DRY $DIM_MUA -s "[PULL] $branch" \
>> +	     -i $req_file "${dim_pull_request_recipients[@]}"
>> +}
>
> s/tags/tag/ everywhere, since it's just a single tag, not plural. That
> also means you can simplify the code a lot. Or I'm missing why you want to
> send a pull request for multiple tags at the same time?
>
> Same for the help text below.
>
> Aside: Autocomplete for this would be _very_ nifty. Should be possible to
> wire up the git tag completion function into our own autocomplete.
>
>> +
>>  # dim_pull_request branch upstream
>>  function dim_pull_request
>>  {
>> diff --git a/dim.rst b/dim.rst
>> index 6d7528ce497f..1a9bed464021 100644
>> --- a/dim.rst
>> +++ b/dim.rst
>> @@ -292,6 +292,12 @@ be merged have been added, in order to help maintainers with deciding which tree
>>  is in need of a pull request. Commiters that want to check the status of their
>>  current branch should use normal **git status** commands.
>>  
>> +pull-request-tags *branch* *upstream* *tags*
>> +--------------------------------------------
>> +Fetches the *upstream* remote to make sure it's up-to-date. Based on the given
>> +*tags*, it generates a pull request template with the specified *upstream*,
>> +and finally is starts \$DIM_MUA with the template with subject and
>> +recipients already set.
>
> I'd elaborate here a bit more.
>
> "This can be used if the final steps of *pull-request* failed, or together
> with *tag-branch* to separate the pull request generation from tag
> creation."
>
> I'd also add a hint to the help text for tag-branch about this new
> command.

So what's the purpose here, really? We're carefully generating a new tag
specifically to avoid re-using existing tags, and now you want to allow
it again? What are these "already existent tag that you just created
outside"?

I've seen these -1 suffix tags from Rodrigo, but I don't recall from
anyone else. What goes wrong? Should we focus on fixing that instead?

BR,
Jani.



>
> Aside from the minor polish lgtm.
> -Daniel
>
>>  
>>  pull-request *branch* *upstream*
>>  --------------------------------
>> -- 
>> 2.17.1
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center