<div dir="ltr"><div dir="ltr">On Tue, 3 Nov 2020 at 10:01, Christian Kühnel <<a href="mailto:kuhnel@google.com">kuhnel@google.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"></div><div class="gmail_quote">Just to be clear: If a patch modified something in <font face="monospace">/llvm</font> and causes a failing test (in <font face="monospace">ninja check-all</font>) in something Tier 2 (e.g. polly), this would count as a failed pre-merge test. Is that what you wanted to have?<br></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>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?</div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>I think this is why we need a tier 2 policy in the first place.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>At the moment pre-merge testing is only reporting results back to Phabricator. AFAIK there is no notification mechanism beyond that.<br></div></div></div></blockquote><div><br></div><div>That's perfectly fine. We don't want to change *anything* that we already do with this policy.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div>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.<br></div></div></div></blockquote><div><br></div><div>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".</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>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.</div><div>With "experimental" you mean code that matches a <font face="monospace">".*/experimental/.*" path regex?</font></div></div></div></blockquote><div><br></div><div>Not exclusively. Also things that don't build or run by default, like new back-ends, new passes, new command line options.</div><div><br></div><div>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.</div><div><br></div><div>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".</div><div><br></div><div>Makes sense?</div><div>--renato</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
</blockquote></div></div>