On Wed, Apr 17, 2019 at 7:20 PM Zebediah Figura z.figura12@gmail.com wrote:
On 04/10/2019 12:40 PM, Vijay Kiran Kamuju wrote:
It seems I have to do a deeper analysis of the code. I checked only whether the tests are successful or not.
For what it's worth (and since the concern was brought up to me offline by someone else), I feel like I should point out that Staging patches are not necessarily correct, either functionally or stylistically. That's the basic reason (even if it's not the only reason) that those patches are in Staging. Hence anyone who's interested in upstreaming Staging patches should treat the patch as their own responsibility, and should ensure that the patch is correct, and ideally have also tested it themself. That's part of what the signoff means. It's also the reason why Alistair and I are slow to upstream patches from Staging. There were a few hundred we tried to push upstream at an early point when we took over the repository, but most of those were the easy ones. We're still stuck with 800 patches which are nontrivial or take a fair amount of background knowledge to understand (like everything involved with ACLs) or, worse, which have no bug report or test application attached to them and thus no way to ensure that they are actually doing the right thing and fixing the problem (or, even more critically, will still be doing the right thing if we make some modifications.)
I am trying to write tests and improve upon or rewrite the patches. For most of the patches tests are present.
Bottom line is, inasmuch as you should be expected to understand and write your own patches and ensure they're correct, the same rules apply to patches someone else wrote. You can't implicitly trust that they're correct, even according to that person's standards, and you certainly can't trust that they're correct just because they're in Staging.
Sometime I try to contact the author, but for some old patches I doubt the author remembers the application or why he/she has written the patch. We have to try to push the patches upstream regardless, as there are no explanations or rejection reasons given. This makes it a bit harder to improve upon the existing patch. The upstreaming is currently a slow process and reviews are incomplete. So I am trying to pick some easy ones and old ones so we get a proper review at least this time. Rejection without proper comments is always a major pain point for Wine.