Add the option to define the target branch on pull requests

[dfinteligenz]

Adds the option to select the target branch of the PR to be created.
In case of not setting the targetBranch option, the behavior is the same as before, it will take the currently selected branch.

An example where the PR will be created with the main branch as the target branch:
cml pr create . --targetBranch=main

[github-actions]

Test Comment

[github-actions]

Test Comment

[tasdomas]

Thank you for the pull request!

I think it can be improved slightly - please see the comments in the change listing

[github-actions]

Test Comment

[github-actions]

Test Comment

[github-actions]

Test Comment

[github-actions]

Test Comment

[dacbd]

g2g https://github.com/iterative/cml-playground/actions/runs/4116875527/jobs/7107481386

functionally works just fine, I think that there might be an edge case worth documenting.

if you are executing this command from branchA targeting branchB your command may pick up unintended changes.

example:

• branchA has some misc changes say related to your workflow.
• ci pipeline starts executing and makes additional changes to the repo
• within branchA your pipeline executes cml pr create --target-branch branchB .

This will pr the changes provided within your pipeline (as expected and standard with other uses) but also all commits up till their shared parent. This may come as a surprise compared to what users might be expecting if they have previous experience with the cml pr create cmd.

---

Where x represents your ci run and perhaps other modifications

    /-b1-b2-(x)
a1-/

using this command with result in:

a1-b1-b2-(x)

this is distinctly different from what users might be expecting from other usage of cml pr expecting something like:

a1-(x)

or

    /-b1-b2-\
a1-/---------\(x)

Hopefully, I have explained this well enough? Playing with it I'm not sure of the intended use case and how this is better than the current behavior (controlling the target branch via the context of the command's execution)

you can see a small example this here: iterative/cml-playground#323 where iterative/cml-playground@d3158ff contains unrelated changes from the context of the ci run and iterative/cml-playground@9bd660b contains the changes from the ci run, both potentially logicly separate sets of changes are include in the same PR.

@dfinteligenz can you confirm this is the behavior you are targeting?
⚠️ marking this as "Request changes" until the author confirms the above

[casperdcl]

I don't follow your diagrams @dacbd.

• using main in lieu of branchA (because idk how to change mainBranchName in GH's mermaid)
• using --rebase merges to clearly show which commits are part of a PR
• assuming auto-deletion post-merge to clean up diagrams

So if you start with:

gitGraph
commit id:"a1"
branch "branchB"
commit id:"b1"
commit id:"b2"

Loading

If b2 triggers a CI job which changes a file then does cml pr --rebase (to create a new commit ci with PR target branchB) you will have:

gitGraph
commit id:"a1"
branch "branchB"
commit id:"b1"
commit id:"b2"
commit id:"ci"

Loading

but if b2 does cml pr --rebase --target-branch=main you will have:

gitGraph
commit id:"a1"
branch "branchB"
commit id:"b1"
commit id:"b2"
commit id:"ci"
checkout "main"
commit id:"b1-copy"
commit id:"b2-copy"
commit id:"ci-copy"

Loading

Are you saying users might be expecting this instead:

gitGraph
commit id:"a1"
branch branchB
commit id:"b1"
commit id:"b2"
commit id:"ci"
checkout main
commit id:"ci-copy"

Loading

... or something else?

[dacbd]

@casperdcl yes, I feel your last case is what users who have used cml pr in the past would naturally expect. However regardless of any options the they specify (--rebase) they will essentially end with the entirety of any changes which already exist in their base branch being included in their PR to the target branch.

I want to confirm this is what the Author wants. If this is the case the PR is g2g.
However, while I initially agreed this seems like a logical option to have, I feel like this is likely to scoop up additional changes that most users might not intend. It might just be my failure of imagination to picture how this new mode of operation is actually useful, as such I just want to confirm this is the intended behavior the Author wants.

[dfinteligenz]

Thanks for the detailed explanation @dacbd, I can confirm that this is indeed the behavior I'm looking for.

Whenever I make some changes and launch a CI job, we would expect the code changes to go along with the data modified by the CI job on the generated PR, not just the changes made on the last commit.

[dacbd]

@dfinteligenz thanks, I will get this merged today. I can't say for certain when we will do another release, but the end of the week or early next week seems reasonable.

[github-actions]

Test Comment

[github-actions]

Test Comment