remotemobprogramming / mob

Tool for smooth git handover.
https://mob.sh
MIT License
1.66k stars 149 forks source link

Automatically Populate Co-Authored-By lines #81

Closed dmcquay closed 3 years ago

dmcquay commented 4 years ago

Because each of the WIP commits is generated by the authors in the group, it is trivial to aggregate the WIP commits and generate a unique list of Co-Author lines. I have written a little wrapper script in bash that does this when I call the done command.

CURRENT_BRANCH=`git rev-parse --abbrev-ref HEAD`
FROM_BRANCH=master
if [[ "$CURRENT_BRANCH" != "mob-session" ]]
then
  FROM_BRANCH=`echo "$CURRENT_BRANCH" | awk -F/ '{print $2}'`
fi

CO_AUTHOR_LINES=$(git log $FROM_BRANCH.. --pretty=format:"Co-Authored-By: %an <%ae>" | sort | uniq | grep -v `git config user.email`)

mob done

printf "\nRecommended Co-Author Lines have been copied to your clipboard:\n\n"
printf "$CO_AUTHOR_LINES"
printf "\n\n\n$CO_AUTHOR_LINES" | pbcopy

I think it would be cool to integrate this into mob done command. I am possibly willing to submit a PR but wanted to get a sense of the maintainers' opinion on this before I put in the effort.

Some things to consider:

MarioKusek commented 4 years ago

@dmcquay This is exactly what I was looking for in the tool.

Regarding auto-populate one way to go is to setup commit template ([https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration] commit.template).

I have done it this way:

  1. setup template git config commit.template .mob-authors - this can be done once on each mobber machine
  2. put .mob-authors file in .gitignore - this can be done once
  3. create file .mob-coauthors with the content of CO_AUTHOR_LINES and put this file in the current directory of repo
  4. when you create commit editor will pick up this file

The drawbacks are:

dmcquay commented 4 years ago

Thanks, Mario! I'll experiment with that in the coming days.

dmcquay commented 4 years ago

Since this is a squash commit, git generates .git/SQUASH_MSG file. When this file is present, the commit template is ignored. So I have to delete that file as part of my script as well.

dmcquay commented 4 years ago

Here's something working just to try out the flow (in bash again).

REPO_ROOT=`git rev-parse --show-toplevel`
CURRENT_BRANCH=`git rev-parse --abbrev-ref HEAD`
FROM_BRANCH=master
if [[ "$CURRENT_BRANCH" != "mob-session" ]]
then
  FROM_BRANCH=`echo "$CURRENT_BRANCH" | awk -F/ '{print $2}'`
fi

CO_AUTHOR_LINES=$(git log $FROM_BRANCH.. --pretty=format:"Co-Authored-By: %an <%ae>" | sort | uniq | grep -v `git config user.email`)

mob done
rm $REPO_ROOT/.git/SQUASH_MSG

printf "message\n\n\n$CO_AUTHOR_LINES" > ~/.gitmob-commit-template

if [[ `git config --global commit.template` != "~/.gitmob-commit-template" ]]
then
        echo "A commit template has been generated with suggested Co-Authors. Use it with this command:"
        echo "git config --global commit.template '~/.gitmob-commit-template'"
fi

Lots of ways we could do this. Perhaps we check if they already have a commit template and append these lines to it?

One downside of this is that the template would remain after the commit and would be stale for non-git-mob future commits. Perhaps we should create a timer background process that removes this template (or removes lines added to an existing template) after some period of time?

I'm going to try this for the next few days or so and see how it feels.

simonharrer commented 4 years ago

Hi, great ideas. I like the idea of automatically appending the co-authors to the .git/SQUASH_MSG via the mob tool. That way, this is non-invasive and users can decide whether to use this or not.

dmcquay commented 4 years ago

Oh geez. That's much better than deleting the SQUASH_MSG and making the user configure a commit template. I will try it that way.

simonharrer commented 4 years ago

Another idea: one could comment the commit messages "mob next [ci-skip]" in the SQUASH_MSG template as well, as no one really wants those messages in the commit in the base branch.

simonharrer commented 3 years ago

Hi @dmcquay any way I can help you with your proposed idea?

dmcquay commented 3 years ago

Absolutely. I have been using my bash implementation of this but I haven’t started on a go implementation at all yet. Feel free to work on it if you want.

On Sat, Nov 14, 2020 at 12:55 PM Simon Harrer notifications@github.com wrote:

Hi @dmcquay https://github.com/dmcquay any way I can help you with your proposed idea?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/remotemobprogramming/mob/issues/81#issuecomment-727257377, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA4Q4OYJIQQR5LY5HOMTH3SP3OBJANCNFSM4SDKW5KA .

stevegt commented 3 years ago

Are there any objections to adding the Co-authored-by: lines as standard behavior? (I think it's the right thing to do.)

Here's what I get when I meld @dmcquay's version with the SQUASH_MSG idea -- not extensively tested yet, but seems to work. We'll use this ourselves here for the next week or so, and if nobody else gets to it first we'll translate to Go and add it to done().

#!/bin/bash -ex

REPO_ROOT=`git rev-parse --show-toplevel`
cd $REPO_ROOT
CURRENT_BRANCH=`git rev-parse --abbrev-ref HEAD`
FROM_BRANCH=master
if [[ "$CURRENT_BRANCH" != "mob-session" ]]
then
  FROM_BRANCH=`echo "$CURRENT_BRANCH" | awk -F/ '{print $2}'`
fi

CO_AUTHOR_LINES=$(git log $FROM_BRANCH.. --pretty=format:"Co-authored-by: %an <%ae>" | grep -v $(git config --get user.email) | sort -u)

mob done

printf "\n\n$CO_AUTHOR_LINES" >> .git/SQUASH_MSG
stevegt commented 3 years ago

I started thinking about how to test this thing, and realized ./create-testbed needs to be refactored to create more local repos and set user.name and user.email on all of them. I've just now done that, added a test case, and pushed to https://github.com/t7a/mob/tree/append-coauthors. I'll watch for pull requests against that branch in case anyone else wants to hack on this. This will eventually come up in our local mob session queue here; we'll add or finish the implementation and send a pull request to @simonharrer (I'll start a draft PR now). I plan to include a coauthor header for @dmcquay regardless, unless Dustin has any objections.

dmcquay commented 3 years ago

Awesome! I’m so glad you’re keeping this moving. I’ll take a look at this in the next day or so.

On Sun, Jan 31, 2021 at 6:58 PM stevegt notifications@github.com wrote:

I started thinking about how to test this thing, and realized ./create-testbed needs to be refactored to create more local repos and set user.name and user.email on all of them. I've just now done that, added a test case, and pushed to https://github.com/t7a/mob/tree/append-coauthors. I'll watch for pull requests against that branch in case anyone else wants to hack on this. This will eventually come up in our local mob session queue here; we'll add or finish the implementation and send a pull request to @simonharrer https://github.com/simonharrer. I plan to include a coauthor header for @dmcquay https://github.com/dmcquay regardless, unless Dustin has any objections.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/remotemobprogramming/mob/issues/81#issuecomment-770512754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA4Q4NUBTRZFZYLIZQWEJTS4YDCVANCNFSM4SDKW5KA .

lamalex commented 3 years ago

144 closes this. it both reads any authors from the squash_msg, as well as appending co-authored-by to WIP commits and making sure those individuals get attribution in the final commit. --with is optional, but enables you to make sure everyone gets attribution.

stevegt commented 3 years ago

We've split the automatic append code off from #144, putting it in #121 to more narrowly close just this bug for the 1.4.0 release, leaving the --with code for later. @dmcquay if you get a chance to exercise the #121 code, that would be great; tests pass and I've pushed as of a few minutes ago, but have no idea yet if it actually works in a real mob. ;-)