[pacman-dev] Submitting Patches
I want to bring this up a little bit. There's lots of ideas thrown around on this list, but more often than not, no code, or cryptic patches. This does nothing. All it does is waste both mine and Dan's time. On 10/24/07, Xavier <shiningxc@gmail.com> wrote:
But if your patch is rejected, then you should probably also provide one separate patch with only the small bug fixes.
This should ALWAYS be the case. Submitting a patch with 5 different changes is NOT acceptable. "One feature per patch" is the way this works. This is not just pacman either. Most other open source projects do it this way. Think of it this way: if you submit a patch with 5 changes, and one change is rejected, then you need to resubmit the whole thing. If you submit 5 patches, and 4 get merged, you only need to resubmit one small patch AND you get listed in the SCM history 4 times. Yay. Secondly, if a patch is submitted, and it's broken, resubmit the patch. Don't say "oh change this". If you'd like a shining example of how this process works, take a look at Nathan's recent patches. He submitted them, in git format, they were commented on, he changed them, resubmitted, and they were merged. It's really that simple. Dan and I do not need all this extra work. What Nathan did is EXACTLY how this process should work (PS Thanks a lot Nathan) Going forward, I think we're going to have to do the following: If you want a change made, a patch MUST be sent in git format. Otherwise it will simply be rejected, not for lack of effort but for the simple fact that it's a waste of our time to have to deal with.
On 10/24/07, Aaron Griffin <aaronmgriffin@gmail.com> wrote:
I want to bring this up a little bit.
There's lots of ideas thrown around on this list, but more often than not, no code, or cryptic patches.
This does nothing. All it does is waste both mine and Dan's time.
On 10/24/07, Xavier <shiningxc@gmail.com> wrote:
But if your patch is rejected, then you should probably also provide one separate patch with only the small bug fixes.
This should ALWAYS be the case. Submitting a patch with 5 different changes is NOT acceptable. "One feature per patch" is the way this works. This is not just pacman either. Most other open source projects do it this way.
Think of it this way: if you submit a patch with 5 changes, and one change is rejected, then you need to resubmit the whole thing. If you submit 5 patches, and 4 get merged, you only need to resubmit one small patch AND you get listed in the SCM history 4 times. Yay.
Secondly, if a patch is submitted, and it's broken, resubmit the patch. Don't say "oh change this".
If you'd like a shining example of how this process works, take a look at Nathan's recent patches. He submitted them, in git format, they were commented on, he changed them, resubmitted, and they were merged. It's really that simple.
Dan and I do not need all this extra work. What Nathan did is EXACTLY how this process should work (PS Thanks a lot Nathan)
Going forward, I think we're going to have to do the following: If you want a change made, a patch MUST be sent in git format. Otherwise it will simply be rejected, not for lack of effort but for the simple fact that it's a waste of our time to have to deal with.
OK, Aaron dropped the ball so I'm going to say whats on my mind as well, hopefully keeping it short. I was going to write an email exactly like this, we both discussed this last night. My new stance- I'm going to make one more pass through my email inbox and see if there are (not-)patches and things that I can easily apply. If so, I'll try to get them at least on a testing branch on my tree. If not, then they are getting deleted. Sorry, but if you can't put it in a form I can easily digest, then I no longer care. Starting NOW, I will no longer even look at an email that is not in GIT patch format if it is supposed to be a "patch". It will go straight to the trash. The rest of you on the list are welcome to take these suggestions and turn them into a GIT patch that I will read, but I'm not going to waste any more time saying "I need to spend 30 minutes looking this over". Nathan's set of 5 patches were an _excellent_ example of what to do. He broke what would have been a large, hard to digest change into 5 discrete patches with the code still compiliing and working after each stage. This is perfect. I don't care if it is a one line change or not- put it in GIT format. This means I want three things- a commit message, your signoff, and the code change itself. That is what has been lacking in a lot of submissions. Signing off makes sure people are actually testing and willing to support their patch. The commit message will end up in the history, so I can reasonably expect it to be a concise definition of the changes. Finally, the code change in patch format is just what I want to be able to see what changed. The *only* other method of patch submission I will accept is having a public GIT tree set up and a 'topull' branch or something similar. However, submissions that should be peer reviewed should still be sent to the mailing list. Why am I being such a hard ass on this? Because I am beginning to lose motivation to merge other peoples changes when it take me more time to figure out how to review them then it did to make the change itself. And I'd like to enjoy working on pacman again. Right now I feel like there is a weight on my shoulders with all these shoddy patches lying around. I want that weight to be gone, and I want others to be able to review patches just as easily as I should. In short- from here on out, only GIT patches accepted. End of story. Happy pacman-ing! -Dan
On 10/24/07, Dan McGee <dpmcgee@gmail.com> wrote:
Why am I being such a hard ass on this? Because I am beginning to lose motivation to merge other peoples changes when it take me more time to figure out how to review them then it did to make the change itself. And I'd like to enjoy working on pacman again. Right now I feel like there is a weight on my shoulders with all these shoddy patches lying around. I want that weight to be gone, and I want others to be able to review patches just as easily as I should.
Well said. Let me add this to. For those of use that code for a living, and do this day in and day out, code is a better explanation than English. Always. Here's an example: a) "Change the index of the loop structure to advance twice as fast" b) - for(int i=0; i<10; i++) + for(int i=0; i<10; i+=2) It's a matter of transferring knowledge. If you want to tell someone something, and it's important, you'll want to transfer knowledge in the best way possible. I'm not going to send someone smoke signals when I can call them on their cell phone, it's inefficient. By the same token, having to convert English text to code is just as inefficient.
Hi! OK. I understood. From now on, I will send only "proper" git patches. But I'm sure that I will send much fewer patches than before. I thought, that my ugly/terrible/no-patch mails were a bit helpful (at least they were bug reports); but if you cannot decode the information from this: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009655.html or from this: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009666.html, I have the feeling, that you simply don't _want_ to decode it. Since _you_ have git access, I don't understand why I should create the patch instead of you, when I gave _all_ the needed information. Yes, I'm lazy; are you lazy too ;-)? To tell the truth; I don't understand you. I tried to help you, but you prevent me from helping with this huge "officialdom". I have the feeling, that I should say thank you, that I'm allowed to send you patches instead of getting "thanks for your effort to help us improve pacman" responses (don't misunderstand me, I do it, because I _like_ hacking it). So from now on I will create my patches with git-format-patch. Bye, ngaba P.S.: Are we allowed to discuss things about development, or are we allowed to send patches only?
On 10/24/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
To tell the truth; I don't understand you. I tried to help you, but you prevent me from helping with this huge "officialdom"
That's just it. You *think* you're helping, but you are not. What you're doing is causing more work for us. Talk is not helpful. Code is. We have a saying over here, that's apt: Put your money where your mouth is. In fact, this is a very common phrase. Simply put: stop talking about it, and do it. I've said it before - we don't need more talk. Talk is easy. I can talk about all the stuff that should be done in pacman too, but that doesn't get it done. We need action.
On 10/24/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
but if you cannot decode the information from this: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009655.html
http://www.archlinux.org/pipermail/pacman-dev/2007-October/009656.html
or from this: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009666.html,
http://www.archlinux.org/pipermail/pacman-dev/2007-October/009668.html
On 10/24/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! OK. I understood. From now on, I will send only "proper" git patches. But I'm sure that I will send much fewer patches than before. I thought, that my ugly/terrible/no-patch mails were a bit helpful (at least they were bug reports); but if you cannot decode the information from this: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009655.html or from this: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009666.html, I have the feeling, that you simply don't _want_ to decode it. Since _you_ have git access, I don't understand why I should create the patch instead of you, when I gave _all_ the needed information. Yes, I'm lazy; are you lazy too ;-)?
And here is where we think differently. No, I don't want to decode it. If it is a problem that you think needs fixing, then test and fix it, please. I don't want to mess up and fix it wrong, and I _really_ don't want to spend duplicate time fixing something you have either already fixed or can fix much quicker than me. Since I have GIT access? Did you miss the concept of a _distributed_ version control system? Everyone has GIT access, this isn't CVS anymore. You can commit a patch just as easily as I. The only difference is that most people tend to follow the Arch Linux pacman tree more so than your tree or my tree. And guess what? A GIT patch gives all the needed information. So why the hell should I have to run it though a translator? That is your job.
To tell the truth; I don't understand you. I tried to help you, but you prevent me from helping with this huge "officialdom". I have the feeling, that I should say thank you, that I'm allowed to send you patches instead of getting "thanks for your effort to help us improve pacman" responses (don't misunderstand me, I do it, because I _like_ hacking it).
We share something in common then- we both like hacking on pacman. And when you do contribute things back to the "official" tree, I will say thanks. There is no trying to stand in your way with "officialdom" here. You are standing in your own way by refusing to admit that someone is going to have to do the work to clean up and format the patch. That isn't my responsibility.
So from now on I will create my patches with git-format-patch.
When you do this, I promise I'll be all eyes and review the patch. I just can't deal with an attached patch followed by a note to fix this and then a note that something else needs to be done first, etc.
Bye, ngaba
P.S.: Are we allowed to discuss things about development, or are we allowed to send patches only?
Of course! I only said that for emails that are supposed to be patches, they have to be in the correct format. If you just want to discuss something, examples in patch format are wonderful because there is no ambiguity (if examples are needed at all). When the discussion becomes a patch however, git-am should be able to deal with it.
participants (3)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor