On 26 November 2017 at 22:30, Glyph <gl...@twistedmatrix.com> wrote: > >> On Nov 26, 2017, at 10:23 AM, Adi Roiban <a...@roiban.ro> wrote: >> >> Hi, >> >> What is the procedure for accepting patches for platforms for which we >> don't have a continuous testing system ? >> >> This is a follow up of the review from >> https://twistedmatrix.com/trac/ticket/9323 [snip] >> > > To support a platform—i.e. to promise we will not break it in the future—we > have to have a builder capable of running Twisted on that platform. > > To accept a patch though, the important thing is to have test coverage. If > the change in question is a platform-specific feature that there’s no way to > test in a different environment, we would indeed need to block on the > availability of a builder. > > Quite often—as I believe is the case for the patch you’re referring to > here—patches are adjusting behaviors which would be difficult to cleanly > integration-test on the platform in question anyway, and the appropriate > thing to do is to make some mock match the observed behavior of the platform > in question. In this case it would be nice if the reviewer could verify > this, but if the patch submitter’s description of observed behavior is clear, > and their test mock implements it correctly, I would say that we can give > them the benefit of the doubt and accept such patches even if the reviewer > doesn’t have such a platform available. Assuming all the changed lines are > covered adequately for our supported platforms’ behavior as well, of course, > and we aren’t risking breaking them. > > How do you feel about this answer?
Thanks for your comment and I think that I know what a reviewer should do for https://twistedmatrix.com/trac/ticket/9323 From the ticket description, we already have a test which fails on Cygwin and the PR does not add any new tests. So the options are: A. Get a builder for cygwin and use the existing tests. B. Write new unit/integration tests with a fake Cygwin system. ----- I understand the high-level process, but I have questions regarding the details. How can a reviewer (which does not have experience and access to an unsupported platform) verify whether the fake/surrogate implementation is correct? That is without putting significant effort and reading the details about the unsupported platform, and in this process becoming an expert in that platform? -- Adi Roiban _______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
