Has anyone meddled with a patch creation form? Eg: 1) select the file of interest 2) paste your variation code 3) submit to create a patch Seems simple, judging from the few times I'm brave enough to use the command line (yes, I'm GUI user who would struggle to code this). In my wanderings through contrib, I've seen many users submit code which solves a problem, but which is not in expected patch format. For example, a zip file of whole modules! This happens more in the contrib modules where patches are less critisized and users are sharing solutions with each other that "just work". Some might question whether we should cater for these developers. I say yes, because they are solving problems but cannot communication the answer very well. If a developer installs a module on a site, and then fixes a bug, then they have a solution - however to then submit that answer in legible form is quite an effort for many. Further more, we then expect users to be able to generate function-contextual patches from root. This is a hard to grapple and I'm sure the effort is too much for some, and they never submit a patch at all. Interested in any thoughts. Simon (sime)
This is actually a very good idea, imo. The best format for this would be to select the drupal version and file that you've changed (or multiple), upload the files, and then have the patch generated. I think this could be done securely as long as the files didn't land in the web directory. This would certainly be an invaluable service. -Robert sime wrote:
Has anyone meddled with a patch creation form? Eg: 1) select the file of interest 2) paste your variation code 3) submit to create a patch
Seems simple, judging from the few times I'm brave enough to use the command line (yes, I'm GUI user who would struggle to code this).
In my wanderings through contrib, I've seen many users submit code which solves a problem, but which is not in expected patch format. For example, a zip file of whole modules! This happens more in the contrib modules where patches are less critisized and users are sharing solutions with each other that "just work".
Some might question whether we should cater for these developers. I say yes, because they are solving problems but cannot communication the answer very well. If a developer installs a module on a site, and then fixes a bug, then they have a solution - however to then submit that answer in legible form is quite an effort for many.
Further more, we then expect users to be able to generate function-contextual patches from root. This is a hard to grapple and I'm sure the effort is too much for some, and they never submit a patch at all.
Interested in any thoughts.
Simon (sime)
On 2/7/07, Robert Douglass <rob@robshouse.net> wrote:
This is actually a very good idea, imo. The best format for this would be to select the drupal version and file that you've changed (or multiple), upload the files, and then have the patch generated. I think this could be done securely as long as the files didn't land in the web directory. This would certainly be an invaluable service.
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development. -- Boris Mann Vancouver 778-896-2747 San Francisco 415-367-3595 Skype borismann http://www.bryght.com
Boris Mann wrote:
On 2/7/07, Robert Douglass <rob@robshouse.net> wrote:
This is actually a very good idea, imo. The best format for this would be to select the drupal version and file that you've changed (or multiple), upload the files, and then have the patch generated. I think this could be done securely as long as the files didn't land in the web directory. This would certainly be an invaluable service.
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development.
True. But I think we could get reasonable patches from such a system, and could do things like strip Windows line breaks, spaces at end of lines, and so forth. We could even run it through the code style script to preempt errors.
Does this need to be integrated into Drupal proper? A "before + after = patch" web form should be not much more complicated than a pastebin. All you need is to have it run on a server that has a cvs-compatible version of diff installed on it. It may even already exist somewhere. That wouldn't handle multiple files very well, but then I don't think any web form could without a great deal of extra complexity. That said, I support the idea as I am often trying to fix things while at work where we have only svn, not cvs, and I haven't been able to roll proper diffs so I've been one of those guilty of posting "add this line here and it fixes it" type pseudo-patches lately. Mea culpa. :-( --Larry Garfield On Wed, 07 Feb 2007 18:27:39 +0100, Robert Douglass <rob@robshouse.net> wrote:
Boris Mann wrote:
On 2/7/07, Robert Douglass <rob@robshouse.net> wrote:
This is actually a very good idea, imo. The best format for this would be to select the drupal version and file that you've changed (or multiple), upload the files, and then have the patch generated. I think this could be done securely as long as the files didn't land in the web directory. This would certainly be an invaluable service.
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development.
True. But I think we could get reasonable patches from such a system, and could do things like strip Windows line breaks, spaces at end of lines, and so forth. We could even run it through the code style script to preempt errors.
Robert Douglass wrote:
Boris Mann wrote:
On 2/7/07, Robert Douglass <rob@robshouse.net> wrote:
This is actually a very good idea, imo. The best format for this would be to select the drupal version and file that you've changed (or multiple), upload the files, and then have the patch generated. I think this could be done securely as long as the files didn't land in the web directory. This would certainly be an invaluable service.
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development.
True. But I think we could get reasonable patches from such a system, and could do things like strip Windows line breaks, spaces at end of lines, and so forth. We could even run it through the code style script to preempt errors.
I believe that people not able and/or willing to provide proper patches will probably only provide second rate code anyway and it isn't worth the bother. Cheers, Gerhard
I believe that people not able and/or willing to provide proper patches will probably only provide second rate code anyway and it isn't worth the bother.
I was one of those people not long ago. Sometimes they fix things that wouldn't be fixed otherwise. At present, if the fix is worthwhile, a more experienced developer may go through the code and create a proper patch. You may be right that it's not worth the trouble to make it easier for inexperienced people to contribute code, but it is worthwhile to make it easier for experienced developers to work on contributed code. Besides, it was an experienced developer who said this system would be easier for him to use.
On Wed, 2007-02-07 at 19:37 +0100, Gerhard Killesreiter wrote:
Robert Douglass wrote:
Boris Mann wrote:
On 2/7/07, Robert Douglass <rob@robshouse.net> wrote:
This is actually a very good idea, imo. The best format for this would be to select the drupal version and file that you've changed (or multiple), upload the files, and then have the patch generated. I think this could be done securely as long as the files didn't land in the web directory. This would certainly be an invaluable service.
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development.
True. But I think we could get reasonable patches from such a system, and could do things like strip Windows line breaks, spaces at end of lines, and so forth. We could even run it through the code style script to preempt errors.
I believe that people not able and/or willing to provide proper patches will probably only provide second rate code anyway and it isn't worth the bother.
I agree with killes. If a user can't diff... I'd expect patches against the wrong version of a module and/or patches that provide poor or incorrect solutions to problems.
Darrel O'Pry wrote:
On Wed, 2007-02-07 at 19:37 +0100, Gerhard Killesreiter wrote:
I believe that people not able and/or willing to provide proper patches will probably only provide second rate code anyway and it isn't worth the bother.
I agree with killes. If a user can't diff... I'd expect patches against the wrong version of a module and/or patches that provide poor or incorrect solutions to problems.
Possibly, but I often get whole modules attached to issues in contrib. Such a tool would at least put an end to that. I also get many "If you add this after line 12345...." issues. Often times, in both cases, the person (arguably with minimal coding skills) has actually identified (and corrected) a real problem. Reviewing and applying patches instead of dealing with the cases I mentioned would be easier. Anyway, it would be cool to build and see what happens. I don't have time to do it.
I agree with killes. If a user can't diff... I'd expect patches against the wrong version of a module and/or patches that provide poor or incorrect solutions to problems.
This provides a 'standard' to diff that even new users who can diff might not know. Windows line breaks, and -p to make things cleaner to read... Heck, sometimes people just forget those and the maintainer gets to re-roll the patch anyway. My windows box I need Cygwin to do a proper command line diff and I need to run it through dos2unix first... this would actually make it easier for some developers. Unless of course, you all are about to point out how I could be doing this better now, :) To me the main value in this is a diff validation of sorts. - Check versions so the module being uploaded _is_ diffed against the proper CVS version. - Auto-remove windows line breaks - Automatically use -p - Automaticaly determine the right place in the directory structure to take the diff from (Would allow multiple file uploads I presume)
Sam Tresler wrote:
- Auto-remove windows line breaks
I just fix the linebreaks and trailing whitespace for every patch I get: wget -qO - $1 | dos2unix | sed -e 's/^\(+.*[^ ]\) *$/\1/' -e 's/^+ *$/+/' -- Neil Drumm http://delocalizedham.com/
Unless of course, you all are about to point out how I could be doing this better now, :) For Windows, TortoiseCVS is a great tool and the latest dev version lets you do diff -up without going to the command line. And I just throw the patches in Notepad++ and convert to Unix line endings. In the near future, TortoiseCVS should have a bit better patch support and maintain line endings well so it will be even easier.
It should be pretty easy to get most Windows users (which might be most users who have trouble patching in the first place) up to speed using TortoiseCVS and Notepad++, both which don't require going to the command line. Maybe I should add a bit more to that TortoiseCVS handbook page... Rob Roy Barreca Founder and COO Electronic Insight Corporation http://www.electronicinsight.com rob@electronicinsight.com Sam Tresler wrote:
I agree with killes. If a user can't diff... I'd expect patches against the wrong version of a module and/or patches that provide poor or incorrect solutions to problems.
This provides a 'standard' to diff that even new users who can diff might not know. Windows line breaks, and -p to make things cleaner to read... Heck, sometimes people just forget those and the maintainer gets to re-roll the patch anyway. My windows box I need Cygwin to do a proper command line diff and I need to run it through dos2unix first... this would actually make it easier for some developers.
Unless of course, you all are about to point out how I could be doing this better now, :)
To me the main value in this is a diff validation of sorts.
- Check versions so the module being uploaded _is_ diffed against the proper CVS version. - Auto-remove windows line breaks - Automatically use -p - Automaticaly determine the right place in the directory structure to take the diff from (Would allow multiple file uploads I presume)
I could see this being a timesaver if it became acceptable practice to link to the generated patch in the issue queue. If you were working on a remote server, you would be spared the extra steps of firing up a FTP client, downloading your patch to a local machine, and browsing for and attaching the patch from the local machine to your patch submission post at the time of submission. Actually, now that I actually type that out, I realize it sounds really lazy. But still, I hate those steps.
Quoting Sam Tresler <sam@treslervania.com>:
My windows box I need Cygwin to do a proper command line diff and I need to run it through dos2unix first... this would actually make it easier for some developers.
When I used to use Cygwin I made sure the mounted directories where in ``binary'' mode so that when I used cvs to check things out I wasn't changing the line ending. I also set the binary switch in vim so that ^M is visable. I don't use Cygwin any longer use MSYS instead (shameless plug). Earnie
Boris Mann wrote:
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development.
We are getting the changes presently anyway. They are just in the form of changed module files that WE have to diff and patch. This has value for simple edits like people who want to take the time to reformat code or change strings, etc. While i agree that /I/ think it is valuable to learn how to "patch dance" i don't want to impose it on contributors who otherwise dont need to. -- Michael Favia michael@favias.org tel. 512.585.5650 http://michael.favias.org
So is someone going to build this now, or do we have to try a reverse bounty? Michael Favia wrote:
Boris Mann wrote:
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development.
We are getting the changes presently anyway. They are just in the form of changed module files that WE have to diff and patch. This has value for simple edits like people who want to take the time to reformat code or change strings, etc. While i agree that /I/ think it is valuable to learn how to "patch dance" i don't want to impose it on contributors who otherwise dont need to.
I will put up US$50 plus offer hosting for the prototype. I know it's not much, but since I'm not CVS/shell experienced enough to code this myself, I better put something where my mouth is. And responding to some other replies. Yes the submitted code might suck, but it's the delivery system we're talking about here. Also, it's simply not right to dismiss the code of people who can't use diff. Before drupal I code VB apps and had never used version control - my lack of formal IT education meant that learning to supply proper patches was *hard*, it took a lot of time, and I had to grok alien concepts, I had to learn how to use shell for feck's sake :-P There are a lot of smart hacks out there who have better things to do than learn CVS. I still want their patches! .s Robert Douglass wrote:
So is someone going to build this now, or do we have to try a reverse bounty?
Michael Favia wrote:
Boris Mann wrote:
I'm all for lowering barriers to entry. I wonder about the quality of code from such a system. Figuring out patch/diff and CVS is good for the soul, not to mention best practices of code development.
We are getting the changes presently anyway. They are just in the form of changed module files that WE have to diff and patch. This has value for simple edits like people who want to take the time to reformat code or change strings, etc. While i agree that /I/ think it is valuable to learn how to "patch dance" i don't want to impose it on contributors who otherwise dont need to.
Quoting sime <info@urbits.com>:
I will put up US$50 plus offer hosting for the prototype. I know it's not much, but since I'm not CVS/shell experienced enough to code this myself, I better put something where my mouth is.
And responding to some other replies. Yes the submitted code might suck, but it's the delivery system we're talking about here.
Also, it's simply not right to dismiss the code of people who can't use diff. Before drupal I code VB apps and had never used version control - my lack of formal IT education meant that learning to supply proper patches was *hard*, it took a lot of time, and I had to grok alien concepts, I had to learn how to use shell for feck's sake :-P
There are a lot of smart hacks out there who have better things to do than learn CVS. I still want their patches!
There are a great number of issues with this not yet given thought to. I'd suggest beginning a g.d.o page so that toughts around what needs to happen can easily be hashed and edited whiteboard style. One issue that I haven't seen discussed is being able to determine the CVS version of the file that needs a difference. You will need the revision number modified to determine the difference with that revision so that the patch can be applied cleanly. If all files have the $Id$ tag the file could be filtered for the tag and the revision of the file expanded from it. But what do you do with the files without the $Id$ tag? Earnie
participants (13)
-
Boris Mann -
Darrel O'Pry -
Darren Oh -
Earnie Boyd -
Gerhard Killesreiter -
Larry Garfield -
Michael Favia -
Neil Drumm -
Rob Barreca -
Robert Douglass -
Sam Tresler -
sime -
William Smith