[kubernetes-sigs/kueue] Change the default merge-method

[trasc]

Change the default merge method for kueue to squash

[trasc]

/cc @alculquicondor
/cc @tenzen-y

[tenzen-y]

@trasc Although I don't have strong objections, what is the motivation?

[tenzen-y]

I'm wondering if we should use the squash method as a default. Because there are situations that the merge method is helpful instead of using the squash method.

[trasc]

@trasc Although I don't have strong objections, what is the motivation?

To skip the squash step in the context of https://github.com/kubernetes/community/blob/5d54720c251baa4f15ec8a14b36e61afd0d48f58/sig-scheduling/CONTRIBUTING.md?plain=1#L62-L63

[trasc]

I'm wondering if we should use the squash method as a default. Because there are situations that the merge method is helpful instead of using the squash method.

The method could be changed for each PR by adding one of the labels:

https://github.com/kubernetes/test-infra/blob/bd6cac2691cb8f0033ad2bf1dcf667aed1b1da58/config/prow/config.yaml#L789-L791C39

[tenzen-y]

https://github.com/kubernetes/community/blob/5d54720c251baa4f15ec8a14b36e61afd0d48f58/sig-scheduling/CONTRIBUTING.md?plain=1#L62-L63

However, the document also says: https://github.com/kubernetes/community/blob/5d54720c251baa4f15ec8a14b36e61afd0d48f58/sig-scheduling/CONTRIBUTING.md?plain=1#L65

[tenzen-y]

I'm wondering if we should use the squash method as a default. Because there are situations that the merge method is helpful instead of using the squash method.

The method could be changed for each PR by adding one of the labels:

https://github.com/kubernetes/test-infra/blob/bd6cac2691cb8f0033ad2bf1dcf667aed1b1da58/config/prow/config.yaml#L789-L791C39

Yes, it's correct. However, I think it is easy to change the merge method to the squash by adding a label rather than changing the merge method to the merge. Since contributors opened the PR need to do some work like organizing and splitting commits, for changing the merge method to merge by adding a label.

[trasc]

However, the document also says: https://github.com/kubernetes/community/blob/5d54720c251baa4f15ec8a14b36e61afd0d48f58/sig-scheduling/CONTRIBUTING.md?plain=1#L65

* Squash the commits when the PR is ready to merge, this does a great favor for the git history.

when the PR is ready to merge is not a real state, I always hope my PR's are ready to merge when I request the review.

[alculquicondor]

What about doing this? #22663

It would allow us to add /approve /lgtm /hold and request a manual squash. The author can squash and release the hold without requiring another lgtm.
A manual squash is sometimes useful when there are way too many commits and the commit message wouldn't be very meaningful, like it was requested here kubernetes/website#42381 (comment)

I know we haven't been very consistent with this, but we could improve.

[alculquicondor]

I guess we can have both
/lgtm
/approve

[trasc]

/assign @spiffxp

[alculquicondor]

Since we still need an approval, can you add kueue to the list for store_tree_hash: true?

[tenzen-y]

It would allow us to add /approve /lgtm /hold and request a manual squash. The author can squash and release the hold without requiring another lgtm.
A manual squash is sometimes useful when there are way too many commits and the commit message wouldn't be very meaningful, like it was requested here kubernetes/website#42381 (comment)

I know we haven't been very consistent with this, but we could improve.

SGTM

[trasc]

Since we still need an approval, can you add kueue to the list for store_tree_hash: true?

done

[alculquicondor]

/hold cancel
/lgtm

[alculquicondor]

/unassign spiffxp
/assign wojtek-t

[wojtek-t]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, trasc, wojtek-t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• config/OWNERS [wojtek-t]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

@trasc: Updated the following 2 configmaps:

• config configmap in namespace default at cluster test-infra-trusted using the following files:
  • key config.yaml using file config/prow/config.yaml
• plugins configmap in namespace default at cluster test-infra-trusted using the following files:
  • key plugins.yaml using file config/prow/plugins.yaml

In response to this:

Change the default merge method for kueue to squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.