[llvm-dev] Policy on support tiers, take 2

[Christian Kühnel via llvm-dev]

Hi Renato,

Thx for the clarification. I think I get your point and I see how
challenging it is to find a good policy there. Thank you for working on
this!



Best,
Christian


On Tue, Nov 3, 2020 at 12:00 PM Renato Golin <rengolin at gmail.com> wrote:

> On Tue, 3 Nov 2020 at 10:01, Christian Kühnel <kuhnel at google.com> wrote:
>
>> Just to be clear: If a patch modified something in /llvm and causes a
>> failing test (in ninja check-all) in something Tier 2 (e.g. polly), this
>> would count as a failed pre-merge test. Is that what you wanted to have?
>>
>
> I think that's the consensus, yes. My take 2 separated projects in an
> attempt to clarify things and only made it worse, ignore that.
>
> Polly and others I listed in tier 2, are a core part of LLVM, even if not
> many people care, and we should not break it.
>
> The reason why I included Polly is because it has some non-LLVM components
> (ex. ISIL) which are not part of LLVM, but that's an orthogonal issue.
>
> How would someone from a Tier 2 project find out that an in-flight patch
>> breaks their Tier 2 project? How should they be notified? How long should
>> the owner of the patch wait before they land it anyway?
>>
>
> That's up to each sub-community. If they want to set a pre-merge check on
> their own areas they need to make sure that, first, it won't stop patches
> from landing (ie. not noisy) and second, their concerns should be voiced
> promptly and we may decide to merge it anyway if the costs of not doing so
> would be much greater.
>
> I think this is why we need a tier 2 policy in the first place.
>
> For example, an important fix needs to land and it builds fine on CMake,
> but not on Bazel, and the fix isn't trivial. Merging the important fix is
> more pressing than having a stable Bazel build. The Bazel sub-community
> will then work to fix that, after the merge.
>
> There may be core (tier 1) fixes in the subsequent Bazel fix, which is
> perfectly fine unless the fix breaks other tier 1 buildbots. The
> sub-community will work and most other people won't even look at the
> reviews, but the buildbots going red means they have to revert the patch
> and try again.
>
> In some cases the sub-community is very responsive and can perhaps fix
> things on the fly, which is fine as per current post-merge review policy,
> but others may be slow, so revert is the safest thing to do. No new
> policies here.
>
> At the moment pre-merge testing is only reporting results back to
>> Phabricator. AFAIK there is no notification mechanism beyond that.
>>
>
> That's perfectly fine. We don't want to change *anything* that we already
> do with this policy.
>
> If you want people (and tools) to behave differently on tier 1 vs. tier 2
>> projects, it should be clear in which category something falls. I agree,
>> the policy should be something generic and high-level so we don't have to
>> change it all the time. However at any given time it should be clear on
>> what the tiers are. If I look at a file in the monorepo, I would like to
>> have an easy way to find out which tier it belongs to.
>>
>
> Right, that's why I started listing things, but that doesn't scale. Some
> people suggested we separate by directory, which is also nice, but could
> end up dividing in weird ways, like "tier1_scripts" vs "tier2_scripts".
>
> One easy way to do that is to link the definition of having "noisy checks"
> to be tier1 and "silent checks" to be tier 2, while silent for the whole
> community doesn't mean silent for all sub-communities.
>
> We already have the policy to create buildbots and have code owners for
> new parts of the code, or when moving things out of experimental, so
> there's a clear promotion into tier 1. And if a sub-community wants to add
> a new noisy check, they need to convince the rest of the community to
> accept that, which essentially means it's not supported by all, and
> therefore, tier 1.
>
> For pre-merge testing, we're only using CMake in the monorepo, so that
>> already has a very narrow scope and excludes some tier 2 projects.
>> With "experimental" you mean code that matches a ".*/experimental/.*"
>> path regex?
>>
>
> Not exclusively. Also things that don't build or run by default, like new
> back-ends, new passes, new command line options.
>
> Those things usually have silent buildbots curated by their
> sub-communities. To move the silent checks to noisy, the sub-community
> needs to promote them out of experimental by consensus of the rest of the
> community, as per the experimental policy, so it's easier to define.
>
> I know it sounds like soft borders will make it hard to understand what is
> what, but this is the actual intent. We can use our other existing policies
> to define how much we collectively care about things, and this policy to
> make explicit what we mean by "care".
>
> Makes sense?
> --renato
>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201103/49e545ce/attachment.html>