Heiko wrote:
This is my first patch using git and also for wine.
Please check it and let me know which things can be made better.
Hi! Welcome to Wine!
You probably need to include a better description, and some conformance tests would help, too. (They should do basic error checking at least, and should skip checks that really need hardware if the required hardware isn't present.)
Also, your patch failed to apply; see http://kegel.com/wine/patchwatcher/results/
You can avoid the particular problem by not including a diff for 'configure' in the patch; the diff for configure.ac suffices.
We have lots of pages (way too many) of advice for how to send patches at winehq.org, but that bit of advice doesn't seem to be in any of them yet :-( http://www.winehq.org/site/sending_patches http://wiki.winehq.org/DeveloperFaq http://winehq.org/site/docs/winedev-guide/codingpractice
It does show up in http://wiki.jswindle.com/index.php/Coding_Hints:Using_Diff
- Dan
On Sat, Aug 23, 2008 at 7:50 AM, Dan Kegel dank@kegel.com wrote:
You can avoid the particular problem by not including a diff for 'configure' in the patch; the diff for configure.ac suffices.
We have lots of pages (way too many) of advice for how to send patches at winehq.org, but that bit of advice doesn't seem to be in any of them yet :-( http://www.winehq.org/site/sending_patches http://wiki.winehq.org/DeveloperFaq http://winehq.org/site/docs/winedev-guide/codingpractice
It does show up in http://wiki.jswindle.com/index.php/Coding_Hints:Using_Diff
and somewhat in http://wiki.winehq.org/Developers-Hints#head-30a7f7f51ce0e42b4141749c9832bae...
Am Samstag, 23. August 2008 16:50:46 schrieb Dan Kegel:
Heiko wrote:
This is my first patch using git and also for wine.
Please check it and let me know which things can be made better.
Hi! Welcome to Wine!
You probably need to include a better description, and some conformance tests would help, too.
As a first description: I am starting with winscard.dll support. I started with this change about two months ago but got distracted from further studying it so I have thought it might be better to offer the so far done stuff - to avoid getting it lost in the flow of time.
I am trying to prepare the winscard.dll stuff for an application written by me. Probably I can extract some smartcard-using code (after my application works) as a conformance test.
The first task to make my application run is being able to list the smartcard readers attached to the system. So far I only get the name of the smartcard reader. But I also need vendor, user friendly name, serial number, model and whether a smartcard is present. Still missing ... This may be due to my patch or to things missing in or not understood in pcsc-lite - not clear so far.
(They should do basic error checking at least, and should skip checks that really need hardware if the required hardware isn't present.)
Also, your patch failed to apply; see http://kegel.com/wine/patchwatcher/results/
You can avoid the particular problem by not including a diff for 'configure' in the patch; the diff for configure.ac suffices.
Okay. Shall I offer a new patch? How to do this then? Is this the only problem with the patch (syntactically)? To mention ... I am a git rookie ...
We have lots of pages (way too many) of advice for how to send patches at winehq.org, but that bit of advice doesn't seem to be in any of them yet :-( http://www.winehq.org/site/sending_patches http://wiki.winehq.org/DeveloperFaq http://winehq.org/site/docs/winedev-guide/codingpractice
It does show up in http://wiki.jswindle.com/index.php/Coding_Hints:Using_Diff
I am going to have a more thorough look into the links.
- Dan
Thanks for your feedback.
Heiko
On Sat, Aug 23, 2008 at 8:45 AM, frechdachs69 frechdachs69@sofortsurf.de
I am trying to prepare the winscard.dll stuff for an application written by me. Probably I can extract some smartcard-using code (after my application works) as a conformance test.
It's a common misconception that tests should be written after the code. In Wine, it's best to write them *before* the code, and get the tests working on Windows. You can even submit the tests first before even starting on the code. Incremental development - improve the tests a bit, then fix the code to pass the tests - is a good way to go. Initial tests can be really trivial, just checking superficial error behavior. See also http://en.wikipedia.org/wiki/Test-driven_development Believe it or not, this makes development go faster!
The first task to make my application run is being able to list the smartcard readers attached to the system. So far I only get the name of the smartcard reader. But I also need vendor, user friendly name, serial number, model and whether a smartcard is present. Still missing ... This may be due to my patch or to things missing in or not understood in pcsc-lite - not clear so far.
That's good to know. Next time you submit the patch, include a three-line summary of the above, say. Also mention which functions you're improving, and how complete they are.
Okay. Shall I offer a new patch? How to do this then? Is this the only problem with the patch (syntactically)? To mention ... I am a git rookie ...
The subject line should have said something like winscard: implement first few functions When you repost after improving a patch, append (try 2) to the subject, e.g. winscard: implement first few functions (try 2)
but I would include a couple tests when you resend; mark them with todo_wine if they don't pass yet on Wine. - Dan
Am Samstag, 23. August 2008 18:06:20 schrieb Dan Kegel: [snip]
Okay. Shall I offer a new patch? How to do this then? Is this the only problem with the patch (syntactically)? To mention ... I am a git rookie ...
The subject line should have said something like winscard: implement first few functions When you repost after improving a patch, append (try 2) to the subject, e.g. winscard: implement first few functions (try 2)
but I would include a couple tests when you resend; mark them with todo_wine if they don't pass yet on Wine.
- Dan
That will take some time since I just had a look at the pcsc-lite stuff ... and its completely missing the information I need in the CCID driver code. So for the special USB smartcard reader in use by me I have to ask the vendor how to retrieve the information ... or otherwise reverse engineer it by sniffing USB while getting these information.
Heiko
-
Dan Kegel -
frechdachs69