* [PATCH 1/3] git pull: remove the existing create_pull_request script
2010-11-06 15:28 [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
@ 2010-11-06 13:35 ` Darren Hart
2010-11-06 13:42 ` [PATCH 2/3] git-pull: add the new create-pull-request script Darren Hart
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Darren Hart @ 2010-11-06 13:35 UTC (permalink / raw)
To: poky; +Cc: Bruce Ashfield
The patches to follow completely rewrite the existing create-pull-request.
Rather than have an initial diff of the two files (which are not at all
similar) remove the original, and then create the new one.
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Nitin A Kamble <nitin.a.kamble@intel.com>
CC: Richard Purdie <rpurdie@linux.intel.com>
CC: Saul Wold <saul.wold@intel.com>
CC: Bruce Ashfield <bruce.ashfield@windriver.com>
---
scripts/create-pull-request | 86 -------------------------------------------
1 files changed, 0 insertions(+), 86 deletions(-)
delete mode 100755 scripts/create-pull-request
diff --git a/scripts/create-pull-request b/scripts/create-pull-request
deleted file mode 100755
index c9a7916..0000000
--- a/scripts/create-pull-request
+++ /dev/null
@@ -1,86 +0,0 @@
-#!/bin/bash
-#
-# create a pull request for your branch
-#
-
-usage() {
- echo "Usage: "
- echo "$ $0 [-r <relative_to>] [-i <commit_id>] -b <contrib_branch>"
- echo " <relative_to> is a commit identifier, like branch-name, HEAD, hex-commit-id"
- echo " <commit_id> is a commit identifier, like branch-name, HEAD, hex-commit-id"
- echo " <contrib_branch> is the branch-name in the git.pokylinux.org/poky-contrib tree"
- echo " If <relative_to> is not specified then relative to master is assumed"
- echo " If <commit_id> is not specified then it is assumed as HEAD"
- echo " For Example:"
- echo " $0 -r master -i misc -b nitin/misc "
- echo " $0 -b nitin/misc "
- echo " $0 -r distro/master -i nitin/distro -b nitin/distro "
- exit 1
-}
-
-while [ $# -ne 0 ] # loop over arguments
-do
-
- case $1 in
- -r )
- shift
- RELATIVE_TO=$1
- shift
- ;;
- -i )
- shift
- COMMIT_ID=$1
- shift
- ;;
- -b )
- shift
- CONTRIB_BRANCH=$1
- shift
- ;;
- *)
- usage
- ;;
- esac
-done
-
-if [ "${COMMIT_ID}" = "" ]; then
- COMMIT_ID=HEAD
- echo "Note: <commit_id> parameter assumed as 'HEAD'"
-fi
-
-if [ "${RELATIVE_TO}" = "" ]; then
- RELATIVE_TO=master
- echo "Note: <relative_to> parameter assumed as 'master'"
-fi
-
-if [ "${CONTRIB_BRANCH}" = "" ]; then
- echo "Error: Parameter <contrib_branch> not specified"
- usage
-fi
-
-git --no-pager show ${COMMIT_ID} > /dev/null
-if [ "$?" != "0" ]; then
- echo "Error: Invalid <commit_id> parameter specified"
- usage
-fi
-
-git --no-pager show ${RELATIVE_TO} > /dev/null
-if [ "$?" != "0" ]; then
- echo "Error: Invalid <relative_to> parameter specified: ${RELATIVE_TO}"
- usage
-fi
-
-echo ""
-git --no-pager diff ${RELATIVE_TO}..${COMMIT_ID} | diffstat -p1
-echo ""
-git --no-pager log --no-merges ${RELATIVE_TO}..${COMMIT_ID} | git --no-pager shortlog
-
-PULL_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=${CONTRIB_BRANCH}"
-
-echo "Pull URL: ${PULL_URL}"
-
-wget -q ${PULL_URL} -O - | grep -q "Invalid branch:\ ${CONTRIB_BRANCH}"
-if [ "$?" == "0" ]; then
- echo "Warning: Branch named '${CONTRIB_BRANCH}' was not found on contrib git tree"
- echo "Check your <contrib-branch> parameter"
-fi
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] git-pull: add the new create-pull-request script
2010-11-06 15:28 [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
2010-11-06 13:35 ` [PATCH 1/3] git pull: remove the existing create_pull_request script Darren Hart
@ 2010-11-06 13:42 ` Darren Hart
2010-11-06 14:06 ` [PATCH 3/3] git-pull: add send-pull-request script Darren Hart
2010-11-07 15:18 ` [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
3 siblings, 0 replies; 8+ messages in thread
From: Darren Hart @ 2010-11-06 13:42 UTC (permalink / raw)
To: poky; +Cc: Bruce Ashfield
The previous create-pull-request only generated a cover letter. When used to
send to the list, it did not include the patches, which made it difficult
to perform peer review. A pull request without patches is typically only sent
by a maintainer. As we are not all maintainers, we need a means to easily
submit patches for review.
As we are accustomed to making pull requests, this script retains a
git-pull-style cover letter, while sending the relevant patches as responses
to the pull. This will provide the necessary context for peer review, and still
allow people to collapse threads and see no more mail than they were previously.
This version retains the relative_to, commit_id, and contrib_branch arguments
from the original, along with their default values. It adds several more,
resulting in a highly flexible tool.
The script creates a pull directory (pull-$$ by default, configurable via the -o
option) and populates it with a git-format-patch generated patch series and
cover letter. The cover letter is modified to include the git and http pull URLs
and branch name, as well as a basic signature from the author pulled from git's
user.name and user.email config. git-format-patch provides the shortlog and
diffstat of the series.
Breaking a bit from the original, this script maintains the [PATCH] subject
prefix in the cover letter (as opposed to [GIT PULL]. This is better suited to
the majority of developers (who are not maintainers). This prefix is
configurable with the -p option, allowing you to create an [RFC PATCH]
prefix, for example.
By default, the generated cover letter with contain "*** SUBJECT HERE ***" and
"*** BLURB HERE ***" tokens which you should replace with something
appropriate prior to sending the messages.
When developing multiple versions of a patch series, it can save time to
maintain a message.txt file, rather than having to retype the message body of
the cover letter every time. The -m option allows you to specify a message file
and replace the "*** BLURB HERE ***" token of the cover letter with the contents
of the message file.
Finally, the -s option will replace the "*** SUBJECT HERE ***" token in the cover
letter with the specified subject.
The generated patches are suitable for sending via sendmail.
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Nitin A Kamble <nitin.a.kamble@intel.com>
CC: Richard Purdie <rpurdie@linux.intel.com>
CC: Saul Wold <saul.wold@intel.com>
CC: Bruce Ashfield <bruce.ashfield@windriver.com>
---
scripts/create-pull-request | 129 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 129 insertions(+), 0 deletions(-)
create mode 100755 scripts/create-pull-request
diff --git a/scripts/create-pull-request b/scripts/create-pull-request
new file mode 100755
index 0000000..5ad7666
--- /dev/null
+++ b/scripts/create-pull-request
@@ -0,0 +1,129 @@
+#!/bin/bash
+ODIR=pull-$$
+RELATIVE_TO="master"
+COMMIT_ID="HEAD"
+PULL_URL="git://git.pokylinux.org/poky-contrib.git"
+PREFIX="PATCH"
+
+usage() {
+CMD=$(basename $0)
+cat <<EOM
+Usage: $CMD [-h] [-o output_dir] [-m msg_body_file] [-s subject] [-r relative_to] [-i commit_id] -b contrib_branch
+ -h Display this help message
+ -o output_dir Specify the output directory for the messages (default: pull-PID)
+ -m msg_body_file The file containing a blurb to be inserted into the summary email
+ -r relative_to Starting commit (default: master)
+ -i commit_id Ending commit (default: HEAD)
+ -b contrib_branch Branch-name in the git.pokylinux.org/poky-contrib tree
+ -s subject The subject to be inserted into the summary email
+ -p prefix Use [prefix N/M] instead of [PATCH N/M] as the subject prefix
+
+ Examples:
+ $CMD -b nitin/basic
+ $CMD -r distro/master -i nitin/distro -b nitin/distro
+ $CMD -r master -i misc -b nitin/misc -o pull-misc
+ $CMD -p "RFC PATCH" -b nitin/experimental
+EOM
+}
+
+# Parse and validate arguments
+while getopts "b:hi:m:o:r:s:p:" OPT; do
+ case $OPT in
+ b)
+ CONTRIB_BRANCH="$OPTARG"
+ ;;
+ h)
+ usage
+ exit 0
+ ;;
+ i)
+ COMMIT_ID="$OPTARG"
+ ;;
+ m)
+ BODY="$OPTARG"
+ if [ ! -e "$BODY" ]; then
+ echo "ERROR: Body file does not exist"
+ exit 1
+ fi
+ ;;
+ o)
+ ODIR="$OPTARG"
+ ;;
+ p)
+ PREFIX="$OPTARG"
+ ;;
+ r)
+ RELATIVE_TO="$OPTARG"
+ ;;
+ s)
+ SUBJECT="$OPTARG"
+ ;;
+ esac
+done
+
+if [ -z "$CONTRIB_BRANCH" ]; then
+ usage
+ exit 1
+fi
+
+
+# Perform a sanity test on the web URL. Issue a warning if it is not
+# accessible, but do not abort as users may want to run offline.
+WEB_URL="http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=$CONTRIB_BRANCH"
+wget -q $WEB_URL -O /dev/null
+if [ $? -ne 0 ]; then
+ echo "WARNING: Branch '$CONTRIB_BRANCH' was not found on the contrib git tree."
+ echo " Please check your contrib-branch parameter before sending."
+ echo ""
+fi
+
+if [ -e $ODIR ]; then
+ echo "ERROR: output directory $ODIR exists."
+ exit 1
+fi
+mkdir $ODIR
+
+
+# Generate the patches and cover letter
+git format-patch -M --subject-prefix="$PREFIX" -n -o $ODIR --thread=shallow --cover-letter $RELATIVE_TO..$COMMIT_ID > /dev/null
+
+
+# Customize the cover letter
+CL="$ODIR/0000-cover-letter.patch"
+(cat <<EOM
+
+Pull URL: $PULL_URL
+ Branch: $CONTRIB_BRANCH
+ Browse: $WEB_URL
+
+Thanks,
+ $(git config user.name) <$(git config user.email)>
+---
+
+EOM
+) | sed -i "/BLURB HERE/ r /dev/stdin" "$CL"
+
+# If the user specified a message body, insert it into the cover letter and
+# remove the BLURB token.
+if [ -n "$BODY" ]; then
+ sed -i "/BLURB HERE/ r $BODY" "$CL"
+ sed -i "/BLURB HERE/ d" "$CL"
+fi
+
+# If the user specified a subject, replace the SUBJECT token with it.
+if [ -n "$SUBJECT" ]; then
+ sed -i -e "s/\*\*\* SUBJECT HERE \*\*\*/$SUBJECT/" "$CL"
+fi
+
+
+# Generate report for user
+cat <<EOM
+The following patches have been prepared:
+$(for PATCH in $(ls $ODIR/*); do echo " $PATCH"; done)
+
+Review their content, especially the summary mail:
+ $CL
+
+When you are satisfied, you can send them with:
+ send-pull-request -a -p $ODIR
+EOM
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] git-pull: add send-pull-request script
2010-11-06 15:28 [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
2010-11-06 13:35 ` [PATCH 1/3] git pull: remove the existing create_pull_request script Darren Hart
2010-11-06 13:42 ` [PATCH 2/3] git-pull: add the new create-pull-request script Darren Hart
@ 2010-11-06 14:06 ` Darren Hart
2010-11-07 15:18 ` [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
3 siblings, 0 replies; 8+ messages in thread
From: Darren Hart @ 2010-11-06 14:06 UTC (permalink / raw)
To: poky; +Cc: Bruce Ashfield
send-pull-request facilitates sending pull requests generated by
create-pull-request. The primary role of this script is to harvest email
addresses from the patches and send them out. A working installation of sendmail
(exim, postfix, msmtp, etc.) is required to use this script.
You can explicitly specify To addresses with the -t option. As this can be
tedious, the -a option will scan all the patches for To, CC, and *-by lines and
the collected addresses to the To and CC headers for each patch.
This script uses an identical recipients list for every patch, including the
cover letter. This is by design. Existing tools will auto-generate the CC header
for individual patches, but since they don't apply it to the other patches, the
recipients can lack the necessary context to provide a meaningful review. This
is especially true of the cover letter.
The pull directory generated by the create-pull-request script is specified
using the -p option.
Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Nitin A Kamble <nitin.a.kamble@intel.com>
CC: Richard Purdie <rpurdie@linux.intel.com>
CC: Saul Wold <saul.wold@intel.com>
CC: Bruce Ashfield <bruce.ashfield@windriver.com>
---
scripts/send-pull-request | 133 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 133 insertions(+), 0 deletions(-)
create mode 100755 scripts/send-pull-request
diff --git a/scripts/send-pull-request b/scripts/send-pull-request
new file mode 100755
index 0000000..0576a5d
--- /dev/null
+++ b/scripts/send-pull-request
@@ -0,0 +1,133 @@
+#!/bin/bash
+AUTO=0
+
+usage()
+{
+cat <<EOM
+Usage: $(basename $0) [-h] [-a] [[-t email]...] -p pull-dir
+ -t email Explicitly add email to the recipients
+ -a Automatically harvest recipients from "*-by: email" lines
+ in the patches in the pull-dir
+ -p pull-dir Directory containing summary and patch files
+EOM
+}
+
+# Collect To and CC addresses from the patch files if they exist
+# $1: Which header to add the recipients to, "TO" or "CC"
+# $2: The regex to match and strip from the line with email addresses
+harvest_recipients()
+{
+ TO_CC=$1
+ REGX=$2
+ export IFS=$',\n'
+ for PATCH in $PDIR/*.patch; do
+ # Grab To addresses
+ for EMAIL in $(sed '/^---$/q' $PATCH | grep -e "$REGX" | sed "s/$REGX//"); do
+ if [ "$TO_CC" == "TO" ] && [ "${TO/$EMAIL/}" == "$TO" ] && [ -n "$EMAIL" ]; then
+ if [ -z "$TO" ]; then TO=$EMAIL; else TO="$TO,$EMAIL"; fi
+ elif [ "$TO_CC" == "CC" ] && [ "${CC/$EMAIL/}" == "$CC" ] && [ -n "$EMAIL" ]; then
+ if [ -z "$CC" ]; then CC=$EMAIL; else CC="$CC,$EMAIL"; fi
+ fi
+ done
+ done
+ unset IFS
+}
+
+
+# Parse and verify arguments
+while getopts "ahp:t:" OPT; do
+ case $OPT in
+ a)
+ AUTO=1
+ ;;
+ h)
+ usage
+ exit 0
+ ;;
+ p)
+ PDIR=${OPTARG%/}
+ if [ ! -d $PDIR ]; then
+ echo "ERROR: pull-dir \"$PDIR\" does not exist."
+ usage
+ exit 1
+ fi
+ ;;
+ t)
+ if [ -n "$TO" ]; then
+ TO="$TO,$OPTARG"
+ else
+ TO="$OPTARG"
+ fi
+ ;;
+ esac
+done
+
+if [ -z "$PDIR" ]; then
+ echo "ERROR: you must specify a pull-dir."
+ usage
+ exit 1
+fi
+
+
+# Verify the cover letter is complete and free of tokens
+CL="$PDIR/0000-cover-letter.patch"
+for TOKEN in SUBJECT BLURB; do
+ grep -q "*** $TOKEN HERE ***" "$CL"
+ if [ $? -eq 0 ]; then
+ echo "ERROR: Please edit $CL and try again (Look for '*** $TOKEN HERE ***')."
+ exit 1
+ fi
+done
+
+
+# Harvest emails from the generated patches and populate the TO and CC variables
+# In addition to To and CC headers/lines, the common Signed-off-by, Tested-by,
+# etc. (*-by) will be added to CC.
+if [ $AUTO -eq 1 ]; then
+ harvest_recipients TO "^[Tt][Oo]: *"
+ harvest_recipients CC "^[Cc][Cc]: *"
+ harvest_recipients CC "^.*-[Bb][Yy]: *"
+fi
+
+if [ -z "$TO" ] && [ -z "$CC" ]; then
+ echo "ERROR: you have not specified any recipients."
+ usage
+ exit 1
+fi
+
+
+# Generate report for the user and require confirmation before sending
+cat <<EOM
+The following patches:
+$(for PATCH in $PDIR/*.patch; do echo " $PATCH"; done)
+
+will be sent to the following recipients:
+ To: $TO
+ CC: $CC
+
+EOM
+echo "Continue? [y/N] "
+read cont
+
+if [ "$cont" == "y" ] || [ "$cont" == "Y" ]; then
+ ERROR=0
+ for PATCH in $PDIR/*patch; do
+ # Insert To and CC headers via formail to keep them separate and
+ # appending them to the sendmail command as -- $TO $CC has proven
+ # to be an exercise in futility.
+ #
+ # Use tail to remove the email envelope from git or formail as
+ # msmtp (sendmail) would choke on them.
+ cat $PATCH | formail -I "To: $TO" -I "CC: $CC" | tail -n +2 | sendmail -t
+ if [ $? -eq 1 ]; then
+ ERROR=1
+ fi
+ done
+ if [ $ERROR -eq 1 ]; then
+ echo "ERROR: sendmail failed to send one or more messages. Check your"
+ echo " sendmail log for details."
+ fi
+else
+ echo "Send aborted."
+fi
+
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 0/3] git-pull: new pull request generation and sending scripts
@ 2010-11-06 15:28 Darren Hart
2010-11-06 13:35 ` [PATCH 1/3] git pull: remove the existing create_pull_request script Darren Hart
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Darren Hart @ 2010-11-06 15:28 UTC (permalink / raw)
To: poky; +Cc: Bruce Ashfield
The following patches replace the existing create-pull-request script and create
a new send-pull-request. See 2/3 for details on the motivation and functionality
of the new create-pull-request script.
Pull URL: git://git.pokylinux.org/poky-contrib.git
Branch: dvhart/git-pull
Browse: http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dvhart/git-pull
Thanks,
Darren Hart <dvhart@linux.intel.com>
---
Darren Hart (3):
git pull: remove the existing create_pull_request script
git-pull: add the new create-pull-request script
git-pull: add send-pull-request script
scripts/create-pull-request | 165 +++++++++++++++++++++++++++----------------
scripts/send-pull-request | 133 ++++++++++++++++++++++++++++++++++
2 files changed, 237 insertions(+), 61 deletions(-)
create mode 100755 scripts/send-pull-request
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] git-pull: new pull request generation and sending scripts
2010-11-06 15:28 [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
` (2 preceding siblings ...)
2010-11-06 14:06 ` [PATCH 3/3] git-pull: add send-pull-request script Darren Hart
@ 2010-11-07 15:18 ` Darren Hart
[not found] ` <4CD782A9.8080201@windriver.com>
3 siblings, 1 reply; 8+ messages in thread
From: Darren Hart @ 2010-11-07 15:18 UTC (permalink / raw)
Cc: Bruce Ashfield, poky
On 11/06/2010 11:28 AM, Darren Hart wrote:
> The following patches replace the existing create-pull-request script and create
> a new send-pull-request. See 2/3 for details on the motivation and functionality
> of the new create-pull-request script.
Thinking about this, I realized that the script would be usefull for
just sending patches as well (for developers without a contrib branch).
Perhaps the scripts should not contain "pull" in their name and the
remote git branch as well as the hosting site should be both optional
and configurable. These are relatively trivial changes which we could
address in a subsequent series if people agree that this initial set is
the right approach for our project.
--
Darren Hart
Embedded Linux Kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] git-pull: new pull request generation and sending scripts
[not found] ` <4CD782A9.8080201@windriver.com>
@ 2010-11-08 15:59 ` Darren Hart
2010-11-09 0:33 ` Saul Wold
0 siblings, 1 reply; 8+ messages in thread
From: Darren Hart @ 2010-11-08 15:59 UTC (permalink / raw)
To: Bruce Ashfield; +Cc: poky
On 11/07/2010 08:55 PM, Bruce Ashfield wrote:
> On 10-11-07 10:18 AM, Darren Hart wrote:
>> On 11/06/2010 11:28 AM, Darren Hart wrote:
>>> The following patches replace the existing create-pull-request script
>>> and create
>>> a new send-pull-request. See 2/3 for details on the motivation and
>>> functionality
>>> of the new create-pull-request script.
>>
>> Thinking about this, I realized that the script would be usefull for
>> just sending patches as well (for developers without a contrib branch).
>> Perhaps the scripts should not contain "pull" in their name and the
>> remote git branch as well as the hosting site should be both optional
>> and configurable. These are relatively trivial changes which we could
>> address in a subsequent series if people agree that this initial set is
>> the right approach for our project.
>
> I saw these in action at the LPC, and I like the two
> phase approach here. Since it gives a chance to edit/update
> review before sending, and like 'git mailinfo' it creates
> the parts you need and doesn't actually do anything
> without a second command :)
>
> Having the patches via email is key for me, so I'm glad
> to see that in place. Do we assume that discretion will
> be used and we don't need a throttle/limit on the patches ?
> But then again, thinking about it, 200+ patch dumps
> to go lkml without the world ending (and it shouldn't
> really be common here) so this probably isn't an issue.
I thought a bit about ensuring proper ordering (I've made scripts wait 5
seconds between patches before). As for abuse... well, I think it is
unlikely - if someone wants to abuse the list, there are innumerable
ways to go about it, this being one of the least efficient ;-)
>
> So definitely acked-by me.
>
Excellent - and this brings up a point I wanted to discuss. One of the
stated reasons for doing this was to facilitate peer review. Now that I
have Bruce's Acked-by:, I could reword the commit messages to include
them, or Richard could add them when he does the pull, but I do think
they should be present in the final commits to poky proper.
Richard - how would you like to see that done? My suggestion would be
that if you accept these as they are, that you append Bruce's Acked-by
and your Signed-off-by. If I end up having to take another pass and send
a V2 pull request, I could confirm Bruce still agrees (in a side
channel) and add his Acked-by.
Thanks,
--
Darren Hart
Embedded Linux Kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] git-pull: new pull request generation and sending scripts
2010-11-08 15:59 ` Darren Hart
@ 2010-11-09 0:33 ` Saul Wold
2010-11-09 6:45 ` Darren Hart
0 siblings, 1 reply; 8+ messages in thread
From: Saul Wold @ 2010-11-09 0:33 UTC (permalink / raw)
To: Darren Hart; +Cc: poky@yoctoproject.org
On 11/08/2010 07:59 AM, Darren Hart wrote:
> On 11/07/2010 08:55 PM, Bruce Ashfield wrote:
>> On 10-11-07 10:18 AM, Darren Hart wrote:
>>> On 11/06/2010 11:28 AM, Darren Hart wrote:
>>>> The following patches replace the existing create-pull-request script
>>>> and create
>>>> a new send-pull-request. See 2/3 for details on the motivation and
>>>> functionality
>>>> of the new create-pull-request script.
>>>
>>> Thinking about this, I realized that the script would be usefull for
>>> just sending patches as well (for developers without a contrib branch).
>>> Perhaps the scripts should not contain "pull" in their name and the
>>> remote git branch as well as the hosting site should be both optional
>>> and configurable. These are relatively trivial changes which we could
>>> address in a subsequent series if people agree that this initial set is
>>> the right approach for our project.
>>
>> I saw these in action at the LPC, and I like the two
>> phase approach here. Since it gives a chance to edit/update
>> review before sending, and like 'git mailinfo' it creates
>> the parts you need and doesn't actually do anything
>> without a second command :)
>>
>> Having the patches via email is key for me, so I'm glad
>> to see that in place. Do we assume that discretion will
>> be used and we don't need a throttle/limit on the patches ?
>> But then again, thinking about it, 200+ patch dumps
>> to go lkml without the world ending (and it shouldn't
>> really be common here) so this probably isn't an issue.
>
> I thought a bit about ensuring proper ordering (I've made scripts wait 5
> seconds between patches before). As for abuse... well, I think it is
> unlikely - if someone wants to abuse the list, there are innumerable
> ways to go about it, this being one of the least efficient ;-)
>
I think we also need a way to only send the initial email (and not the
patches) since I work with the team to pull together multiple patches
that are already on the list and re-send the request to the Yocto alias,
I don't want to flood the list with duplicate match info.
Also on a minor nit note: can you change the Branch to include contrib/
that way it can be a cut and paste for doing some of the local git
operations.
>>
>> So definitely acked-by me.
>>
>
> Excellent - and this brings up a point I wanted to discuss. One of the
> stated reasons for doing this was to facilitate peer review. Now that I
> have Bruce's Acked-by:, I could reword the commit messages to include
> them, or Richard could add them when he does the pull, but I do think
> they should be present in the final commits to poky proper.
>
I am not sure we need to get into updating the patch or commit messages
with Acked-by lines, the email archives should suffice.
Sau!
> Richard - how would you like to see that done? My suggestion would be
> that if you accept these as they are, that you append Bruce's Acked-by
> and your Signed-off-by. If I end up having to take another pass and send
> a V2 pull request, I could confirm Bruce still agrees (in a side
> channel) and add his Acked-by.
>
> Thanks,
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] git-pull: new pull request generation and sending scripts
2010-11-09 0:33 ` Saul Wold
@ 2010-11-09 6:45 ` Darren Hart
0 siblings, 0 replies; 8+ messages in thread
From: Darren Hart @ 2010-11-09 6:45 UTC (permalink / raw)
To: Saul Wold; +Cc: poky@yoctoproject.org
On 11/08/2010 04:33 PM, Saul Wold wrote:
> On 11/08/2010 07:59 AM, Darren Hart wrote:
Hey Saul,
Thanks for the review!
> I think we also need a way to only send the initial email (and not the
> patches) since I work with the team to pull together multiple patches
> that are already on the list and re-send the request to the Yocto alias,
> I don't want to flood the list with duplicate match info.
This is really no different that it was originally. Just load the
0000-cover-letter.txt into your favorite MUA and send away.
Alternatively, you can just delete the other messages and use
send-pull-request.sh.
I'd rather not make an option that makes it easy for people to not send
the patches, as it may become the norm (as that is the habit people are
already in). Will one of the options I mentioned work for you?
>
> Also on a minor nit note: can you change the Branch to include contrib/
> that way it can be a cut and paste for doing some of the local git
> operations.
The name "contrib" is user defined and cannot be relied upon to be the
same on every system. The branch is relative to the poky-contrib.git
repository, prepending "contrib" wouldn't be accurate.
>> Excellent - and this brings up a point I wanted to discuss. One of the
>> stated reasons for doing this was to facilitate peer review. Now that I
>> have Bruce's Acked-by:, I could reword the commit messages to include
>> them, or Richard could add them when he does the pull, but I do think
>> they should be present in the final commits to poky proper.
>>
> I am not sure we need to get into updating the patch or commit messages
> with Acked-by lines, the email archives should suffice.
It can be difficult to map commits to email threads, and when the ack
occurs elsewhere, the history is completely lost. We don't need to track
every "I like this patch", but I believe it is good practice to track
key Acks from key reviewers.
Thanks,
--
Darren Hart
Embedded Linux Kernel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-11-09 6:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-06 15:28 [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
2010-11-06 13:35 ` [PATCH 1/3] git pull: remove the existing create_pull_request script Darren Hart
2010-11-06 13:42 ` [PATCH 2/3] git-pull: add the new create-pull-request script Darren Hart
2010-11-06 14:06 ` [PATCH 3/3] git-pull: add send-pull-request script Darren Hart
2010-11-07 15:18 ` [PATCH 0/3] git-pull: new pull request generation and sending scripts Darren Hart
[not found] ` <4CD782A9.8080201@windriver.com>
2010-11-08 15:59 ` Darren Hart
2010-11-09 0:33 ` Saul Wold
2010-11-09 6:45 ` Darren Hart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.