On Apr 24, 2022 at 4:38 AM Nikolay Sivov nsivov@codeweavers.com wrote:
On 4/24/22 11:48, Austin English wrote:
On Sat, Apr 23, 2022, 07:17 Nikolay Sivov nsivov@codeweavers.com
wrote:
On 4/23/22 11:57, Mohamad Al-Jaf wrote:
On Sat, Apr 23, 2022 at 4:01 AM Nikolay Sivov nsivov@codeweavers.com
wrote:
What I mean is, have you checked that?
Why would that be necessary? It's clear that the game tries to load mshtmlmedia under Windows 7 so why wouldn't this be the case in Wine running in Windows 7 mode?
Testing on wine would be necessary to see if it's needed.
I have to agree with Mohamad here. AFAIK, it's never been a requirement that a patch actually be verified before committing it:
- I don't see it mentioned on
https://wiki.winehq.org/Submitting_Patches
- There are over a thousand bugs that have been closed fixed that
contain
the comment 'Should be fixed by':
https://bugs.winehq.org/buglist.cgi?bug_status=CLOSED&limit=0&list_i...
(I realize that isn't an accurate count, but demonstrates my broader
point).
---Sorry for preemptive send---
FYI, I was trying to comment constructively, not aggressively.
Your point is that there are more CLOSED/FIXED reports that don't have "should be fixed by" ? One way to improve this situation was to link bug urls in commit messages. We should definitely continue improving it, for better traceability.
I agree, we should absolutely try to improve commit messages/link bug reports appropriately.
If you mean that "should be fixed" translates to "I think it might be
fixed, but I'm not sure", in my experience it usually means the opposite, when commenter actually tested it one or another to make sure.
Sometimes it does (which is why the overall number isn't accurate), but a lot of developers also use more definitive phrases like 'this is fixed by XXXXX'. I also recall several bugs that were commented/closed as 'should be fixed by ..' and then reopened after testing confirmed it hadn't been fixed..and for reasons that the author could/should have realized. Generally for bugs that have phrases like 'this is fixed by XXXXX' don't have that mistake.
If there's constructive feedback that hasn't been given, by all means,
give
it, but "it hasn't been explicitly tested" isn't a reason to block in standard wine development. Maybe we should revisit that, but until we
do, I
don't think it should block this patch.
It depends really. In this case it's some internal IE11 (or maybe a bit older) module, with only justification that some users, running actual Windows 7 installation, found it useful. Second example is that it was mentioned in chromium sources somewhere. Is it unreasonable to discuss if it's useful for Wine?
No, I think it's absolutely reasonable to question it, to a point. But given that the codebase shows it's possible, and that there are public reports showing that it happens is reasonable evidence.
At the same time, I agree of course that just the contributor's desire
to work on something is enough to consider submitted work. For this patch what kind of feedback would you expect?
I think the feedback/review, in general, has been fair/valid. I am disagreeing in how explicit Wine is being, as a project, for testing patches before submitting upstream.
P.S. needless to say, my replies are my own, I don't have the power to reject submitted work, nor would I like to have it.
I agree that _actually_ testing would be ideal, but I don't think it should be a _blocker_. That said, I'm in the same position, and my position is my own. I raise it because I feel like few others were giving feedback :).