Need feedback on first four UTF-7 patches

Sebastian Lackner sebastian at fds-team.de
Wed Oct 29 15:37:16 CDT 2014


On 29.10.2014 20:50, Alex Henrie wrote:
> 2014-10-29 6:45 GMT-06:00 Michael Stefaniuc <mstefani at redhat.com>:
>> But the code still could look a lot better and some of the previous
>> feedback wasn't addressed. There is an impedance mismatch between the
>> code looking good for Alex and looking good for Wine.
> 
> As far as I know, I only rejected one of your suggestions for these 4
> patches: Merging the simple_test and complex_test structs. The 3
> simple tests are actually used as inputs for 8 tests each, for a total
> of 24 tests. Reformatting the 3 simple tests as complex tests would
> force me to manually input the 24 combinations into the complex_tests
> array. Is that really what Alexandre wants?
> 
> -Alex
> 
> 

It is not really true that this was the only thing we criticized. We also gave you a lot more suggestions for improvements, but from the beginning I had the impression that you want to keep changes as minimal as possible. Thats not a good attitude when trying to upstream code, and will probably be especially a problem for the implementation itself. You always have to consider multiple methods how a specific problem can be solved.

For the tests: The main issue (bad test coverage) is fixed at my opinion, but there are still lot of small things which could be improved to make the code more readable. Most of it was already told on IRC, but I will go over patch 1 one more time and tell you again.




More information about the wine-devel mailing list