For future reverts, please link to the tracking bug, and give a general description of why we're reverting the PR.

Looking at b/290302660 indicates this requires a g3fix, and @justinmc has prepped it. Can you try applying the patch to your roll and seeing if that fixes it?

For future reverts, please link to the tracking bug, and give a general description of why we're reverting the PR.

Looking at b/290302660 indicates this requires a g3fix, and @justinmc has prepped it. Can you try applying the patch to your roll and seeing if that fixes it?

Just did and it is still failing

@justinmc Sorry but I find this a bit offensive for the contributor. I'm sure there's some effort that had gone into this, and by the looks of it the tests were not failing.

Only hidden ones that are closed off failed? Seems a bit unfair, doesn't it? Do we know why it failed? I feel like this is how you create a scenario where fewer people want to contribute.

@CaseyHillers @Jasguerrero

@justinmc Sorry but I find this a bit offensive for the contributor. I'm sure there's some effort that had gone into this, and by the looks of it the tests were not failing. Only the hidden, closed off tests that nobody can actually work with did?

Seems a bit unfair, doesn't it? Do we know why it failed? I feel like this is how you create a scenario where fewer people want to contribute.

@CaseyHillers @Jasguerrero

The contributor reach out on discord and I explained the situation, there is a new effort on relanding this already #130171

Sorry if there were any confusion

@vleeuwenmenno There's nothing personal about reverts, they happen all the time and are not in any way a comment on the person writing the code. It's just part of the process. I myself get PRs reverted all the time. :-) It's so common that the word "revert" appears 44 times just on our "tree hygiene" wiki page, including on the first line. :-)

We accept private test suites from anyone who has the resources to hook into our CI and maintain them, currently only Google has taken us up on that but anyone is welcome to do it. We get additional significant testing that way to help keep the product high-quality, and they get stability (because we don't break them). The trade-off is that they must promise to follow-up on failures, either working with the person who wrote the patch, or fixing their tests (as is happening in this case in #130171).

@justinmc Sorry but I find this a bit offensive for the contributor. I'm sure there's some effort that had gone into this, and by the looks of it the tests were not failing. Only the hidden, closed off tests that nobody can actually work with did?
Seems a bit unfair, doesn't it? Do we know why it failed? I feel like this is how you create a scenario where fewer people want to contribute.
@CaseyHillers @Jasguerrero

The contributor reach out on discord and I explained the situation, there is a new effort on relanding this already #130171

Sorry if there were any confusion

Are there any changes to the code that fix the cause of failing tests? Because now the git blame will not be accurate to the original author of the contributed code.

Thanks @Hixie, I was going to write something similar.

@vleeuwenmenno You are right that I should have communicated this better though. I've actually already got another PR open to try to reland @gilnobrega's work (#130171). I have a fix that I'll apply to the failing Google tests. Saying "No big deal for this to be reverted" was a bad choice of words :). I meant that there aren't any sequential PRs that depend on this one that will be broken by a revert etc., not that the PR is unimportant.

Are there any changes to the code that fix the cause of failing tests? Because now the git blame will not be accurate to the original author of the contributed code.

No changes need to be made to the original PR to fix the tests. I'm happy to close my PR if @gilnobrega wants to open their own. I think the original commits will still be attributed to @gilnobrega either way at least.

@vleeuwenmenno There's nothing personal about reverts, they happen all the time and are not in any way a comment on the person writing the code. It's just part of the process. I myself get PRs reverted all the time. :-) It's so common that the word "revert" appears 44 times just on our "tree hygiene" wiki page, including on the first line. :-)

We accept private test suites from anyone who has the resources to hook into our CI and maintain them, currently only Google has taken us up on that but anyone is welcome to do it. We get additional significant testing that way to help keep the product high-quality, and they get stability (because we don't break them). The trade-off is that they must promise to follow-up on failures, either working with the person who wrote the patch, or fixing their tests (as is happening in this case in #130171).

Thanks for the clarification, although I do not quite agree with that work flow but that's alright. Keeps the way of working challenged to be better 😃

@justinmc @Hixie Thank you for the explanation.

As @vleeuwenmenno mentioned, as far as I'm aware a revert of a revert erases git blame so I reopened the PR in #130173.

I'm happy to deliver improvements if needed.

Thanks! I've closed mine in favor of yours.