On Fri Jan 31 17:26:57 2025 +0000, eric pouech wrote:
Thanks for the submission. I'm not thrilled by the outcome style-wise, but given the starting point, I fully agree with you that it'll way too large to go further. Two remarks though: The commit message should read: "programs/cmd: Cleanup DIR /O logic." (without the quotes) we want to trace which module is addressed by the change, and commit line should be terminated by a '.' Content-wise, we definitively need to embed the tests (as the ones I posted above) inside the merge request in order to detect potential further regressions. This means the MR shall contain two patches:
- first patch in MR: the tests (marked with @todo_wine@ for the failing
ones),
- the second patch in MR: basically your current patch + removal of
@todo_wine@ that now pass. Either you feel up to do it (feel free to use the patch of tests above); if not, I can repackage your patch into the desired form.
Thanks for the feedback and guidance. I appreciate it.
I can certainly try to change the commit message and add the test patch to the MR. I was never able to get the tests to run on my system, however. I believe that it's likely that my build is inadequate. I am not building a full Wine with WOW64 support, etc. Perhaps the CI will run the tests and perhaps I can see the results to see if they are failing, etc.