Re: [OE-core] RFC: create-pull-request / send-pull-request updates
On Wed, May 11, 2011 at 9:15 AM, Darren Hart dvh...@linux.intel.com wrote: Between myself and others, there are several outstanding proposals to modify the pull-request scripts. Patches have been sent, but nothing has been merged due to a lack of consensus. I thought I would summarize what I see to be the current weaknesses of the scripts and my proposal to address them. I would like your feedback to ensure we have tools that meet the needs of a broad user base. Once we agree, I'll be happy to write up the patches or help review those written by others. 1) send-pull-request is too aggressive with the auto-cc feature (Khem Raj) One of the reasons I wrote these scripts was out of frustration with git-send-email which allowed for a cover letter, but didn't include all the collected addresses on it. The current script sends every message to all the collected addresses. Khem (and others) have found this behavior to be sub-optimal, if not down right annoying. I propose it be modified to only use the addresses local to each patch for the patches and all the collected addresses only for the cover letter. a) Do people agree with this policy? If not, and people prefer the cover-letter only be sent to the recipients specified on the command line, then this script doesn't add any real value over 'git request-pull' and 'git send-email', and users can easily wrap those on their own. 2) create-pull-request needs to facilitate the use of multiple repositories (Tom Rini) Some folks find gitorious or github work best for their use. It is also reasonable to want to use this script with independent layers. Tom proposed a -l option to specify the PULL_URL leaving some boiler-plate text in the cover-letter for the user to populate. This dovetails with something I've been considering. Rather than duplicate the generation of a pull request cover letter, I'd like to see us re-use the output of 'git request-pull'. This has the added benefit of sanity checking the URL and commits. It does however remove the concept of the BROWSE_URL. We could add the BROWSE_URL back for recognized locations (git.yoctoproject.org, gitorious, and github I believe). Some have expressed a desire for the URL to be automatically discoverable. We could try and extract this information from the current branch and a remote of a specific name, with some URL rewrites to convert ssh access to generic git access. Unfortunately, this approach breaks under several conditions. I would prefer that these scripts not be tied to any particular naming conventions for the git branches or remotes. I propose we go with something very similar to Tom's -l PULL_URL proposal and replace the cover letter generation with the output of 'git request-pull'. The PULL_URL should also be able to be specified via an environment variable. Now that we are sending patches for review and not just the pull request itself, I feel we can drop the BROWSE_URL. 3) Rely on git-send-email exclusively (Darren Hart) When I originally wrote these scripts, not all the users were particularly familiar with git and others may have already had a local sendmail client configured. At the time I thought it prudent to decouple the mail process from git. In retrospect, this serves only to unnecessarily complicate the send script as users must all learn git to effectively work within the OE and Yocto environments. If a local MTA is available, git can be configured to use that. There is no advantage that I can see to maintain both sending mechanisms in the scripts. They add complexity and complicate debugging and maintenance. I propose the local sendmail mechanism be removed from send-pull- request and that it rely exclusively on 'git send-email'. 3) Rewrite the scripts in python (Tom) While I agree that anything of any significant complexity is better written in python than bash, I feel that with the above changes, the current scripts will be smaller and remain simple enough for bash to be a viable option. I propose we leave the scripts in bash for the time being, leaving the door open to rewrite them at a later date should their complexity increase to merit the effort. Thoughts/Comments? I would suggest to alter the process a bit and get rid of the scripts completely. Patches are sent to mailing list for review once reviewed the final patches are sent as git pull-request. It would simplify things. -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] RFC: create-pull-request / send-pull-request updates
On Wed, 2011-05-11 at 10:01 -0700, Khem Raj wrote: On Wed, May 11, 2011 at 9:15 AM, Darren Hart dvh...@linux.intel.com wrote: Thoughts/Comments? I would suggest to alter the process a bit and get rid of the scripts completely. Patches are sent to mailing list for review once reviewed the final patches are sent as git pull-request. It would simplify things. I'd argue that it doesn't. It just means the requests come in different formats, sometimes with key pieces of information missing which means the people trying to handle the requests (like me) get frustrated. I find it easiest to deal with requests that have come through those scripts. Cheers, Richard ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] RFC: create-pull-request / send-pull-request updates
On 05/11/2011 10:40 AM, Richard Purdie wrote: On Wed, 2011-05-11 at 10:01 -0700, Khem Raj wrote: On Wed, May 11, 2011 at 9:15 AM, Darren Hart dvh...@linux.intel.com wrote: Thoughts/Comments? I would suggest to alter the process a bit and get rid of the scripts completely. Patches are sent to mailing list for review once reviewed the final patches are sent as git pull-request. It would simplify things. I think I know where you're coming from Khem, and I don't disagree that it would simplify things for some people. However, we have a wide range of people working on the various portions of the project. The scripts offer a means of standardizing how patches are reviewed and pulled, and I think that has improved things significantly over how things were a year ago. With the Linux kernel, the vast majority of patches are just sent to the list as email. Pull requests are typically sent from sub-maintainers. However, the poky developers have long been using a pull model for many contributors. The problem was the patches weren't ever hitting the list. This led me to write the scripts in the first place. They attempted to maintain the pull model which worked well for the maintainers while still ensuring there was easy access to the patches for review. I'd argue that it doesn't. It just means the requests come in different formats, sometimes with key pieces of information missing which means the people trying to handle the requests (like me) get frustrated. I find it easiest to deal with requests that have come through those scripts. Obviously we need to try and make things as easy as possible for the maintainers to merge in changes. One thing I think would be painful for maintainer with the current model, is that a pull request appears (at least to me) to be the final version of a patch series, when in fact they can be the very first iteration and still require review. The pull does make it easy to do some testing of patches in addition of review though. I can certainly see both sides to this. Cheers, Richard -- Darren Hart Intel Open Source Technology Center Yocto Project - Linux Kernel ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] RFC: create-pull-request / send-pull-request updates
On 05/11/2011 09:15 AM, Darren Hart wrote: Between myself and others, there are several outstanding proposals to modify the pull-request scripts. Patches have been sent, but nothing has been merged due to a lack of consensus. I thought I would summarize what I see to be the current weaknesses of the scripts and my proposal to address them. I would like your feedback to ensure we have tools that meet the needs of a broad user base. Once we agree, I'll be happy to write up the patches or help review those written by others. Thanks for taking this up! [snip] 2) create-pull-request needs to facilitate the use of multiple repositories (Tom Rini) So long as (a) it's supported and (b) it's easy to use, I'm fine with however you want to implement it :) Which brings me to... [snip] 3) Rewrite the scripts in python (Tom) While I agree that anything of any significant complexity is better written in python than bash, I feel that with the above changes, the current scripts will be smaller and remain simple enough for bash to be a viable option. I propose we leave the scripts in bash for the time being, leaving the door open to rewrite them at a later date should their complexity increase to merit the effort. To me, dealing with some sort of prefs file means non-bash. But if you can figure out everything that's needed with a little bit of asking git and a little bit of standard-shell-magic (which it sounds like you can), yeah, keeping it in bash is fine. -- Tom Rini Mentor Graphics Corporation ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] RFC: create-pull-request / send-pull-request updates
On Wed, May 11, 2011 at 13:15, Darren Hart dvh...@linux.intel.com wrote: 2) create-pull-request needs to facilitate the use of multiple repositories (Tom Rini) Some folks find gitorious or github work best for their use. It is also reasonable to want to use this script with independent layers. Tom proposed a -l option to specify the PULL_URL leaving some boiler-plate text in the cover-letter for the user to populate. This dovetails with something I've been considering. Rather than duplicate the generation of a pull request cover letter, I'd like to see us re-use the output of 'git request-pull'. This has the added benefit of sanity checking the URL and commits. It does however remove the concept of the BROWSE_URL. We could add the BROWSE_URL back for recognized locations (git.yoctoproject.org, gitorious, and github I believe). Some have expressed a desire for the URL to be automatically discoverable. We could try and extract this information from the current branch and a remote of a specific name, with some URL rewrites to convert ssh access to generic git access. Unfortunately, this approach breaks under several conditions. I would prefer that these scripts not be tied to any particular naming conventions for the git branches or remotes. I propose we go with something very similar to Tom's -l PULL_URL proposal and replace the cover letter generation with the output of 'git request-pull'. The PULL_URL should also be able to be specified via an environment variable. Now that we are sending patches for review and not just the pull request itself, I feel we can drop the BROWSE_URL. I'd prefer to have it stored into git config backend and don't need to retype it for every call. -- Otavio Salvador O.S. Systems E-mail: ota...@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Re: [OE-core] RFC: create-pull-request / send-pull-request updates
On Wed, May 11, 2011 at 10:40 AM, Richard Purdie richard.pur...@linuxfoundation.org wrote: On Wed, 2011-05-11 at 10:01 -0700, Khem Raj wrote: On Wed, May 11, 2011 at 9:15 AM, Darren Hart dvh...@linux.intel.com wrote: Thoughts/Comments? I would suggest to alter the process a bit and get rid of the scripts completely. Patches are sent to mailing list for review once reviewed the final patches are sent as git pull-request. It would simplify things. I'd argue that it doesn't. It just means the requests come in different formats, sometimes with key pieces of information missing which means the people trying to handle the requests (like me) get frustrated. I find it easiest to deal with requests that have come through those scripts. I think review process and pull process are different. and pull follows reviews right now they are sort of mixed. There are multiple pull requests that are sent and sometimes one patch appears in multiple pull requests. Signoffs acks etc. are not collected. Thats why I was suggesting to use emails for reviews and there will be trail and then create pull requests Cheers, Richard ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
