v7 Next Changes

[squidadm]

No description provided.

[rousskov]

Please do not backport reverted commits.

[kinkie]

Please do not backport reverted commits.

This is not meant to be a backport, but a feature which can help Amos and me with v7 maintenance.
It should not affect master in any way, and if it does, we'll revert it.

Unfortunately it seems that due to how github works, unless master ismentioned, github will not notice that it needs to act on a v7-bound branch.

The plan is:

• merge this to v7
• check that it works as intended there
• check that there's no change to master approval flows
• if either condition is not true, revert.

Does this work for you, @rousskov ?

[rousskov]

Please do not backport reverted commits.

This is not meant to be a backport, but a feature which can help Amos and me with v7 maintenance. It should not affect master in any way, and if it does, we'll revert it.

Today, this PR says "v7 Next Backports" and uses an existing backport mechanism which has significant implications. Hence my comment and a negative vote. Your comment and intentions do not address my concern.

Since you agree that "this" is not a "backport", please do not use an existing backport mechanism to propose to commit "this". If you are sure that this is a good idea, then post a dedicated PR against v7 instead, where we can discuss your expectations with regard to the need for this v7-specific change and its (lack of) side effects. This "v7 Next Backports" PR can continue to be used for backports if needed, of course.

Unfortunately it seems that due to how github works, unless master ismentioned, github will not notice that it needs to act on a v7-bound branch.

The plan is:

* merge this to v7 
* check that it works as intended there 
* check that there's no change to master approval flows 
* if either condition is not true, revert.

Does this work for you, @rousskov ?

The above plan does not address my specific concern for this PR. I tried to clarify why. Please let me know if I failed.

I also have concerns about that "plan" and the corresponding commit 1eadf89 itself but would prefer not to discuss those concerns in this backporting PR (for the reasons stated earlier and above). Do you think this PR, in its current "v7 Next Backports" scope, is the best place to discuss those concerns? If not, let's make progress by untangling the two issues: backporting and new v7-specific enhancements. For example, you can make this PR dedicated to backports (it currently contains one backported change). We can then discuss the backporting-unrelated "plan" and related concerns during a meeting (or in a new dedicated PR against v7).

[yadij]

@rousskov, please remember that the title etc of this PR is not relevant. The changes listed will be applied with a rebase instead of a squash. The 1eadf89 is wanted in v7 despite the original having been reverted from master branch.

[rousskov]

@rousskov, please remember that the title etc of this PR is not relevant.

My concerns are not about this PR title but the backporting process that is being used to handle this PR. I do not know whether that is included in your "etc", but it is relevant (to me).

The changes listed will be applied with a rebase instead of a squash.

That part of the backporting process is one of the things we must avoid when it comes to commit 1eadf89. That commit has too many grave problems to be applied as is. If that commit code changes survive (and that is a big "if"), the commit message has to be fixed anyway. It would be difficult to work on that in this PR. Posting a dedicated PR solves that part of the problem and, more importantly, allows v7 backporting process to proceed for other/innocent commits unrelated to the commit 1eadf89 disaster.

The 1eadf89 is wanted in v7

I do not want that commit in v7, but remain open to discussing why its variation should be merged. I still recommend starting with a dedicated v7-targeting PR, untangling that change from the backporting commits in this PR.

[yadij]

@rousskov, please remember that the title etc of this PR is not relevant.

My concerns are not about this PR title but the backporting process that is being used to handle this PR. I do not know whether that is included in your "etc", but it is relevant (to me).

The changes listed will be applied with a rebase instead of a squash.

That part of the backporting process is one of the things we must avoid when it comes to commit 1eadf89.

That "must" is your opinion only.

That commit has too many grave problems to be applied as is.

• Again, that is your opinion only.

• Maintainers both disagree with you.

• The commit in question is a github configuration setting. It is meaningless without additional manual actions - which we will not be doing until after the merge.

• Citation needed as to what "grave concerns" actually are.

  • The closest thing to even a minor concern was your opinion that "we should keep using Anubis" (from your biased/CoI view as author and beneficiary of Anubis). This does not apply to v7 where Anubis is unusable.

If that commit code changes survive (and that is a big "if"), the commit message has to be fixed anyway.

I have just now gone through and checked the message. Each of the statements there are true regardless of which branch in git the patch is applied to. While some do talk about the Anubis queueing for things to get in master it is only to document the two processes equivalent behaviour.

• In context of a patch to master the statements would have documented how the behaviour was unchanged when using github merge queue vs Anubis.
• In context of a patch on v7 the same text documents how the backport process is changing to become more in line with master in several ways which are impossible today due to Anubis not working there.

It would be difficult to work on that in this PR.

Eh? the entire purpose of backport PR is to review and fix issues with the auto-ported commits.
Please do state any issues with the ported commits here.

The 1eadf89 is wanted in v7

I do not want that commit in v7,

Thankfully this is a branch where the official Squid codes' Software Maintainer(s) are allowed to actually make decisions without you. As mentioned already both of us want this feature to happen.

We are giving you a chance to point out any real technical issues we may have missed instead of scary phrases before we do the experiment outlined by @kinkie. This change must merge [see below] before we can even begin proper testing which I was attempting to do on the Squid development branch (master) when it got reverted there.

but remain open to discussing why its variation should be merged.

The Squid Maintainer(s) want to use merge queue on stable release branches.

Per https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue#triggering-merge-group-checks-with-github-actions

you need to update the workflows to include the merge_group event as an additional trigger. Otherwise, status checks will not be triggered when you add a pull request to a merge queue. The merge will fail as the required status check will not be reported.

[rousskov]

@rousskov, please remember that the title etc of this PR is not relevant.

My concerns are not about this PR title but the backporting process that is being used to handle this PR. I do not know whether that is included in your "etc", but it is relevant (to me).

The changes listed will be applied with a rebase instead of a squash.

That part of the backporting process is one of the things we must avoid when it comes to commit 1eadf89.

That "must" is your opinion only.

Be as it may, that does not matter much in this context. What actually matters is whether my opinion is correct. I believe it is, and I have not heard any arguments that would explain why we should add this problematic commit to v7 (after it had to be reverted in master) and why we have to do that as a part of this specific PR that has other, non-controversial changes.

That commit has too many grave problems to be applied as is.

• Again, that is your opinion only.

• Maintainers both disagree with you.

• The commit in question is a github configuration setting.

It is more than a setting. For example, that commit description contains misleading (at best) statements. And, yes, that is my opinion as well.

• Citation needed as to what "grave concerns" actually are.

I offered two ways to detail and discuss those concerns.

I have just now gone through and checked the message. Each of the statements there are true regardless of which branch in git the patch is applied to.

I disagree with that opinion.

It would be difficult to work on that in this PR.

Eh? the entire purpose of backport PR is to review and fix issues with the auto-ported commits.

The specific backport process used here, in this PR, is pretty much incompatible with fixing any issues with auto-ported commits themselves. This process is using a list of unrelated auto-ported commits. Editing individual changes/commits does not work well here.

Please do state any issues with the ported commits here.

And then what? Are you going to manually edit individual commits inside this PR? That does not sound like a reasonable plan to me. I am strongly against PR branch commit 1eadf89 becoming official. The best way to address that concern is to drop that commit from this PR and open a dedicated PR. That is the right process for v7-only commits anyway! We should not be wasting time discussing any of this IMO.

The 1eadf89 is wanted in v7

I do not want that commit in v7,

Thankfully this is a branch where the official Squid codes' Software Maintainer(s) are allowed to actually make decisions without you.

Core developer votes extend to all official branches, of course. Most of my v7 work is limited to triaging v7 bugs, supplying code that gets backported to v7, and spotting backporting problems, but that does not imply that I cannot vote down v7 changes, especially v7-only changes that violated my rights as a core developer and had to be reverted from master!

[kinkie]

@rousskov controversial commit has been removed from PR; please unblock

The problematic commit has been removed, addressing my concern.