Thanks Caron, that's exactly the kind of feedback I wanted.
I now feel really stupid. The spacing and line breaks comes from copy/pasting the patch into an email, as I couldn't get git-send-email to work. Apparently, it has it's own package in Ubuntu. That's causing the linebreaks. The spacing I cannot explain, but ofcourse both come out clean when using git-send-email. Thanks for that. That's still no excuse for sending a patch with bad formatting though. Now I just have to figure out how to properly use git-send-email so I can add the comments I want. I'm new to git.
Do I need to sign off my own patches? Just curious how it works, I had imagined one of the proper developers had to sign it off.
--
Actually, you sent it to the right place. There are some things you should have a look at that my eyes are drawn to.
First, when reviewed here, the patch has odd line breaks and spacing:
https://source.winehq.org/patches/data/121371
It's also missing your signed-off-by line. You can read about how to add that here:
https://wiki.winehq.org/Submitting_Patches
Your patch should come through clean and match the surrounding style. I had a patch accepted adding the same number of lines in the same general places, pay close attention to how the two differ in appearance:
https://source.winehq.org/patches/data/121336
I'm looking at those patches simply by going to this page to make sure everything "looks" okay:
https://source.winehq.org/patches/
Note the red "X" indicating the missing sign-off next to your patch.
You would do yourself many favors by using git send-email to send patches or in the very least, take a moment to create a patch and check it over once, twice, three times, before sending. I commit to my local tree and then I do:
git format-patch origin
and compare what's going on and then I make sure Wine still builds after my changes. I catch a lot of mistakes that way (like missing a comma...).
Thank you for sending a patch and following up on it. Others might have more feedback for you.
-Caron
No problem, the learning curve is a little steep when you first start out. I'm still learning, see what I did:
https://source.winehq.org/patches/data/121067 (scroll to the bottom, it's pretty nasty looking. The beginning of the patch was okay but the extra noise made the patch less than ideal. For what it's worth, my email client ate the patch, hence the friendly reminder that I should be using git send-email)
Unless you are only sending your patch to get comments on what you're still working on (those should go to wine-devel), you should always sign-off on your own work. This means that you have reviewed it and believe it ready for The Wine Project. Basically, it means you are willing to work on this should you cause a regression. As already mentioned, adding "-s" will put the sign-off in place.
Then, once your patch passes the automatic tests, the expert in the area will be assigned to review it (in this case, Henri Verbeet). Pay careful attention to their review and address all of the concerns they express over your patch. Provide a response or example where necessary to show your patch is correct OR correct the patch based on their feedback.
When the reviewer believes it ready, they sign-off, like this:
https://source.winehq.org/patches/data/121376
That means everything checks out and the reviewer also believes it ready. This makes Alexandre's role as the maintainer easier. He will have final say on whether the patch goes in. Be careful with your Julliard Rank:
https://wiki.winehq.org/Developer_FAQ#I_sent_a_patch.2C_but_it_got_ignored._...
Good Luck!
-----Original Message----- From: wine-devel [mailto:wine-devel-bounces@winehq.org] On Behalf Of Kim Malmo Sent: Monday, April 18, 2016 12:03 PM To: wine-devel@winehq.org Subject: wined3d: Add Nvidia GT 9700M
Thanks Caron, that's exactly the kind of feedback I wanted.
I now feel really stupid. The spacing and line breaks comes from copy/pasting the patch into an email, as I couldn't get git-send-email to work. Apparently, it has it's own package in Ubuntu. That's causing the linebreaks. The spacing I cannot explain, but ofcourse both come out clean when using git-send-email. Thanks for that. That's still no excuse for sending a patch with bad formatting though. Now I just have to figure out how to properly use git-send-email so I can add the comments I want. I'm new to git.
Do I need to sign off my own patches? Just curious how it works, I had imagined one of the proper developers had to sign it off.