On Mon, 27 Aug 2018, Daniel Vetter <[email protected]> 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 <[email protected]>
>> Cc: Daniel Vetter <[email protected]>
>> Cc: Jani Nikula <[email protected]>
>> Signed-off-by: Rodrigo Vivi <[email protected]>
>> ---
>>  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
_______________________________________________
dim-tools mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dim-tools

Reply via email to