Test-infra: Tide labels to override for all merge methods

Created on 21 Mar 2019  路  10Comments  路  Source: kubernetes/test-infra

Right now, it's possible to use tide/squash to manually override the repository's merge method and squash instead:
https://github.com/kubernetes/test-infra/blob/master/prow/tide/tide.go#L821-L829

In our use, we generally want to squash by default but occasionally override to rebase & keep the individual commits.

areprotide good first issue help wanted kinfeature lifecyclactive

Most helpful comment

Instead, we should honor a tide/rebase label that overrides a config-set default of squash for a PR.

Exactly. Today tide/squash ignores the default and overrides to squash. There should be a tide/rebase to override and do that. And similarly, a tide/merge (except that it's an ugly name :/ ) to do _that_.

I personally don't care about that last one, but having support for 2/3 of the options is even weirder than today's support for 1/3.

All 10 comments

/area prow/tide
/cc @cjwagner

/help
/good-first-issue

@cjwagner:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/help
/good-first-issue

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.

馃憢 Hello!

I was looking to work on this issue, but wanted to clarify slightly before getting started.

Is the proposal to change the default merge_method for all repositories from merge to squash and then instead of a SquashLabel have a RebaseLabel to override the default for a particular commit (instead of overriding the merge method for the repository as a whole)?

Or should we leave the default merge, allowing users to override that at the repository level with squash in the config, but then also have a RebaseLabel to be used on specific commits?

@cji hey! Today, getting the default to squash is as simple as changing the configuration, so I don't think we want to change the default. Instead, we should honor a tide/rebase label that overrides a config-set default of squash for a PR.

@stevekuznetsov awesome, will work on implementing that. Thanks for the help clarifying!

Instead, we should honor a tide/rebase label that overrides a config-set default of squash for a PR.

Exactly. Today tide/squash ignores the default and overrides to squash. There should be a tide/rebase to override and do that. And similarly, a tide/merge (except that it's an ugly name :/ ) to do _that_.

I personally don't care about that last one, but having support for 2/3 of the options is even weirder than today's support for 1/3.

/lifecycle active

Good luck! May be worth a quick Google doc or issue explain how you intend to solve this before starting on the PR (vs just sending over the solution)

Cool, thanks for the suggestion @fejta! - I started riffing on a couple of options here

:+1:

Thanks @cji!

Was this page helpful?
0 / 5 - 0 ratings