Hello,
I have a patchset in queue for about a month now, and I wondered what happened to it.
Is it that you just didn't find time to review them or that they're "not obviously right" like the wiki states? Is there something I can do to help here, so my patchsets don't go unreviewed for weeks?
Regards, Fabian Maurer
Fabian Maurer dark.shadow4@web.de writes:
Hello,
I have a patchset in queue for about a month now, and I wondered what happened to it.
It wouldn't hurt to mention which patchset you mean ;-)
I'm guessing you mean the environment variable thing. It was already rejected, as a test that requires a reboot is not useful to have in the tree, and it's not clear that there's anything to fix since it doesn't work so well in Windows either according to the bug reporter. You'd need to make a more convincing case that we really need this.
Hello Alexandre,
thanks for your response. Yes I'm talking about "ntdll: Add test for environment variables in PEB block". I wasn't aware it was rejected, so I assumed it's just that there is doubt that an interactive test like this is useful.
It stopped with my mail "I'd still prefer to have it in the tests, that way it's easy available if you want to run it. I mean, it's an interactive test, they are meant to be run on demand, no?" and the patchtracker didn't say anything either. I still would consider a test case that's on demand to be worthy of inclusion. It's there if anyone wants to confirm the behavior, easier than having to pull it from some bugreport.
it's not clear that there's anything to fix since it doesn't work so well in
Windows either according to the bug reporter. You'd need to make a more convincing case that we really need this.
Well, it works in a predictable (yet inconsistent) behavior on windows, as my test proves. There is a usecase that works on windows, but not on wine, so I consider that enough to warrant a change. If you don't consider an edge case like this important enough though, then that's how it is, and I'll drop it.
Regards, Fabian Maurer
Fabian Maurer dark.shadow4@web.de writes:
Hello Alexandre,
thanks for your response. Yes I'm talking about "ntdll: Add test for environment variables in PEB block". I wasn't aware it was rejected, so I assumed it's just that there is doubt that an interactive test like this is useful.
I believe I had marked it as rejected after the first comments, but maybe that was for the first version of the patch.
Well, it works in a predictable (yet inconsistent) behavior on windows, as my test proves. There is a usecase that works on windows, but not on wine, so I consider that enough to warrant a change. If you don't consider an edge case like this important enough though, then that's how it is, and I'll drop it.
I don't think that single test case is enough to determine the exact Windows behavior, and I don't see a need to add complexity if the behavior is still not correct after the change. But I don't think either that it's worth investigating in more detail until we find an app that depends on this.
Hello Alexandre,
But I don't think either that it's worth investigating in more detail until we find an app that depends on this.
Well, it's a usecase that works on windows but does not on wine. Why do we need an app, isn't a workflow enough? See https://bugs.winehq.org/ show_bug.cgi?id=46901. If tests is the problem, I could add more.
Regards, Fabian Maurer
Fabian Maurer dark.shadow4@web.de writes:
Hello Alexandre,
But I don't think either that it's worth investigating in more detail until we find an app that depends on this.
Well, it's a usecase that works on windows but does not on wine. Why do we need an app, isn't a workflow enough? See https://bugs.winehq.org/ show_bug.cgi?id=46901. If tests is the problem, I could add more.
It would need more tests to determine the exact Windows behavior, but honestly, given how trivial it is to adapt the workflow to work on both platforms I don't think it's worth the trouble.
-
Alexandre Julliard -
Fabian Maurer