[PATCH] dim: allow to add link to the current HEAD

[Lucas De Marchi]

On Mon, Nov 18, 2019 at 12:16:19PM -0800, Rodrigo Vivi wrote:
>     On Nov 18, 2019, at 12:14 PM, Lucas De Marchi
>     <[1]lucas.demarchi at intel.com> wrote:
>     On Mon, Nov 18, 2019 at 04:57:34PM +0200, Jani Nikula wrote:
>
>       On Fri, 15 Nov 2019, Lucas De Marchi <[2]lucas.demarchi at intel.com>
>       wrote:
>
>         Add method add-link-head that applies the link to head without any
>         additional check. This is useful either to reduce typing or to add a
>         link while in the middle of an interactive rebase.
>
>       I see the patch does what it says on the box. However:
>
>       1) Most commands expect you to supply the branch name so you know what
>        you're doing. There's tab completion to aid you, but you need to see
>        what branch you're operating on. This has been in place since there
>        was only one gun and two feet; now there are quite a few more people
>        with opportunities to shoot themselves in the foot.
>
>       2) I think we should discourage pushing locally rebased
>        patches. Recommend only pushing patches that apply cleanly. I'd go as
>        far as removing the add-link* subcommands altogether.
>
>     Not sure how this is helping or avoiding that. "push" was never
>     mentioned here and it's not what this is about.
>
>     This was useful for me while rebasing a "feature branch"
>     to prep a new revision.  It's useful for the future developer reading
>     the patch to have a link to the revision where the patch was discussed,
>     rather than a link to a last revision where all the reviews were already
>     collected.
>
>   Very good point. But we need to do in a way that we don't loose the
>   last link. The last one is the proof that it passed CI.

If you later apply the patch using, say, "dim aq" a second link will be
added, the link that is already there won't be replaced.

I think the link to the review is much more useful than the link where
CI reported a pass. Either because of CI flip flops or because a patch
that passed CI being applied on top of 10 additional commits doesn't
prove anything.

Anyway, if people just put the automatically-added "Link" when you
download a patch from patchwork, I will just stop having more work on my
end and follow that.

Lucas De Marchi

>
>     Also any patch being applied to any branch are in other words being
>     rebased since your branch probably changed from the time you wrote it
>     and the time it was applied, even if it applies cleanly (your rebase
>     might have being clean, too).
>
>     Lucas De Marchi
>
>       BR,
>       Jani.
>
>         Signed-off-by: Lucas De Marchi <[3]lucas.demarchi at intel.com>
>         ---
>         dim | 23 +++++++++++++++--------
>         1 file changed, 15 insertions(+), 8 deletions(-)
>
>         diff --git a/dim b/dim
>         index 1c2da80..3aad25f 100755
>         --- a/dim
>         +++ b/dim
>         @@ -1255,17 +1255,11 @@ function dim_rebase
>         fi
>         }
>
>         -function dim_add_link
>         +function dim_add_link_head
>         {
>         -  local branch file message_id
>         +  local file message_id
>
>         -  branch=${1:?$usage}
>         -  shift
>         file=$(mktemp)
>         -
>         -  assert_branch $branch
>         -  assert_repo_clean
>         -
>         cat > $file
>
>         message_id=$(message_get_id $file)
>         @@ -1279,6 +1273,19 @@ function dim_add_link
>         fi
>         }
>
>         +function dim_add_link
>         +{
>         +  local branch file message_id
>         +
>         +  branch=${1:?$usage}
>         +  shift
>         +
>         +  assert_branch $branch
>         +  assert_repo_clean
>         +
>         +  dim_add_link_head
>         +}
>         +
>         function dim_add_link_queued
>         {
>         dim_add_link drm-intel-next-queued "$@"
>
>       -- 
>       Jani Nikula, Intel Open Source Graphics Center
>
>     _______________________________________________
>     dim-tools mailing list
>     [4]dim-tools at lists.freedesktop.org
>     [5]https://lists.freedesktop.org/mailman/listinfo/dim-tools
>
>References
>
>   Visible links
>   1. mailto:lucas.demarchi at intel.com
>   2. mailto:lucas.demarchi at intel.com
>   3. mailto:lucas.demarchi at intel.com
>   4. mailto:dim-tools at lists.freedesktop.org
>   5. https://lists.freedesktop.org/mailman/listinfo/dim-tools