On Wed, Jul 10, 2024 at 12:11 PM Kyle Lippincott <spectral@xxxxxxxxxx> wrote: > > On Tue, Jul 9, 2024 at 3:50 PM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote: > > +* If you rely on some configuration or behavior, add a test for it. You may > > +find it easier to add a unit test ensuring the behavior you need than to add an > > +integration test; either one works. Untested behavior is subject to breakage at > > +any time. > > Should we state that we reserve the right to reject these tests if > they would put what we feel is an excessive burden on the git > developers? i.e. a requirement to use C89 would be rejected > (obviously). a requirement to support 16-bit platforms would also be > rejected. I don't know that we need to list examples for what we'd > reject, they could be implied that we're likely to accept anything > else. brian mentioned something similar in their review, to which I proposed a minimum requirements field[1]; I think this would work for that too? Thoughts? > > +** Clearly label these tests as necessary for platform compatibility. Add them > > +to an isolated compatibility-related test suite, like a new t* file or unit test > > +suite, so that they're easy to remove when compatibility is no longer required. > > +If the specific compatibility need is gated behind an issue with another > > +project, link to documentation of that issue (like a bug or email thread) to > > +make it easier to tell when that compatibility need goes away. > > I think that we likely won't have the ability to investigate whether > it's _truly_ gone away ourselves, and there's no guarantee that the > person that added these tests will be able to vet it either (maybe > they've switched jobs, for example). > > I think we should take a stance that may be considered hostile, but I > can't really think of a better one: > - there needs to be a regular (6 month? 12 month? no longer than 12 > month surely...) reaffirmation by the interested party that this is > still a requirement for them. This comes in the form of updating a > date in the codebase, not just a message on the list. If this > reaffirmation does not happen, we are allowed to assume that this is > not needed anymore and remove the test that's binding us to supporting > that. I like the idea of the date in code, because that's "polled" rather than "pushed" - when I break the test, I can go look at it, and see a condition like "Until July 1st, 2024" and say "well it's July 11th, so I don't care, deleted!" - or, more likely, I can see that date expired a year and a half ago :) > We should probably go a step further and intentionally violate > the test condition, so that any builds done by the interested parties > break immediately (which should be caught by the other processes > documented here; if they don't break, then it was correct to remove > the restriction). ...but this seems harder to keep track of. Where are we remembering these "due dates" and remembering to break them on purpose? I'm not sure that there's a good way to enforce this. > - _most_ of these restrictions should probably have a limited number > of reaffirmations? I feel like this needs to be handled on a > case-by-case basis, but I want us to be clear that just because we > accepted these restrictions in the past doesn't mean we will accept > them indefinitely. > - Just because there's a reaffirmation doesn't mean we're guaranteeing > we won't delete the test before the affirmation "expires". If there's > an urgent security need to do something, and it can't be done without > breaking this, we'll choose to break this test. If there's general > consensus to do something (adopt a new language standard version, for > example), there's no guarantee we'll wait until all the existing > affirmations expire. I honestly like this point more than the previous one. Limiting by a number, even one that changes, feels less flexible than allowing ourselves to say "enough of this nonsense, it's the Century of the Fruitbat, we really want to use <C11 feature> so you can get maintenance updates instead now". > > The thinking here is that this test is imposing a restriction on the > git developers that we've agreed to take on as a favor: we are going > to restrict ourselves from doing X _for the time being_ not > necessarily because it'll break you, but because it's a bad experience > for the git developers to create a patch series that lands and then > gets backed out when the breakage on $non_standard_platform is > detected on seen/next/master. 1: https://lore.kernel.org/git/CAO_smVjZ7DSPdL+KYCm2mQ=q55XbEH7Vu_jLxkAa5WTcD9rq8A@xxxxxxxxxxxxxx/T/#m9d7656905be500a97ee6f86b030e94c91a79507a
