Want to be able to change "finish" commands default behavior to "Create a Pull Request"

[ycamargo]

I'm looking for a good set of tools to support our team in using Git Flow branching strategy integrated with our TFS (and soon VSTS) branch policies enforcements.

Today, we're doing it manually. Using basic git tools for create, fetch, pull, push, etc our feature/release/hotfix branches based on develop or master. And we're using policies on our develop and master branches that enforces the code review and approval before merging. Those policies also requires the creation of "Pull Requests" to update those branches.

We would love to simplify the process (and make it less error prone) using one of the GitFlow tools implementation. Since we all use Visual Studio 2017, your extension would be the perfect choice. The only problem at our sight is with "feature/release/hotfix finish" commands. In our strategy, this command should never be used since we need to create a new pull request instead of local merging. This article explains a similar scenario: http://www.clemensreijnen.nl/post/2015/07/22/Gitflow-and-VSO-branch-policies-use-them-together

Would be very interesting if we could config the extension to: commit last changes, push them and open the web browser at the Create Pull Request page (as the TeamExplorer extension does) when developer choose the "finish command". Other alternative could be, at least, the ability to disable those commands in the extension, forcing developers to not finish their branches, avoiding local merges to develop/master.

[arnoldpistorius]

+1

[olljanat]

I found that there is new VSTS CLI tool provided by Microsoft which can be used as "backend" to provide this functionality: https://docs.microsoft.com/en-us/cli/vsts/get-started

So rest is that someone just have time to implement it.

[ycamargo]

It's also possible to call the TFS/VSTS REST API to create the Pull Request (https://www.visualstudio.com/en-us/docs/integrate/api/git/pull-requests/pull-requests#create-a-pull-request). Are you accepting PRs for this project? I could help to implement these features.

[olljanat]

Yes, that VSTS CLI also uses REST API on background.

I'm not contributor on this project but @jakobehn approved at least my pull request (#66) couple months ago.

[jakobehn]

@ycamargo I'm definitely accepting PR's to this project, sorry for being absent. Do you still have time to help out with this?

[ycamargo]

@jakobehn I will try to implement it but, because I'm a little busy right now, don't expect anything so soon.

[jakobehn]

@ycamargo No worries, I'm looking at picking this one up, so check back before you start working on this. Thanks!

[clintirving]

@jakobehn - I'd be thrilled to see some incarnation of this enhancement as well. If the repo has a restriction to only allow a merge to the develop branch via a pull request and you happen to check all 3 checkboxes on the finish step (delete server feature branch, delete local feature branch, rebase from develop) your changes are lost with no feedback/warning.

[mattkoch614]

+1!

[adamlonsdale]

I would also find this very useful.

[schnitty]

+1 Definitely need a gitflow that is compatible with pull requests. I too never use the finish commands

[schnitty]

Is anyone actively implementing this? If not I may take a look.

[ycamargo]

Hi @schnitty , Unfortunatelly, the only thing I did was to fork the repo. Right now, no one is working on this. You'll be very welcome to start it.

[jakobehn]

@schnitty Please go ahead and grab this one, I cleared myself from the assignee list :-)

[schnitty]

Before heading down a particular path I'd like to propose a few options for the implementation and get some feedback.

Option 1: Config option to disable finish commands Have GitFlow with AzureDevOps PR config option. When enabled would simply make the finish commands unavailable which removes the biggest danger. You could then use the PR features from team explorer/web portal to do what you need to do manually.

Advantages

• Team Explorer already has the create PR
• Quick and easy to implement

Disadvantages

• Have to perform some steps manually (creating multiple PRs for release and hotfix could be bothersome)
• Have to remember to delete local branches

Option 2: New PR command to occur before finish command Introduce a PR/publish command that would create pull request(s) accordingly. Modify finish command so it can only be done when the attached PR is completed / abandoned. Finish command would then simply clean up branches (merge would be done by PR)

Advantages

• All in one solution

Disadvantages

• Take longer to implement, test and get right

At this stage i'm thinking it would be at least a good idea to do option one so that the finish commands are disabled.

Then could extend that option as a separate PR to see if we can get the full round trip working from the extension.

I think I prefer the introduction of a new command (gitflow PR) instead of re-purposing the finish command. The feature/release/hotfix isn't finished until the PR is complete.

Thoughts?

[olljanat]

@schnitty how about using option 1 together with https://marketplace.visualstudio.com/items?itemName=sander-aernouts.git-buildtasks extension?

I added example screenshots to https://github.com/sanderaernouts/vsts-git-tasks/issues/2

[schnitty]

@olljanat Thanks for the recommendation. I don't think however that I would want to create a PR as part of a build pipeline.

I'm pretty sure that I'll be able to create a PR using one of the nuget packages listed here: https://docs.microsoft.com/en-us/azure/devops/integrate/concepts/dotnet-client-libraries?view=vsts

[jakobehn]

@schnitty My vote would be for option 2. I realize that this is more work of course, but for me one of the core values of this extension is to not having to leave VS for the basic workflows.

But if it turns out to be a very heavy item to implemente, I'm definitely fine with option 1 as a start of course

[ycamargo]

@olljanat Thanks for the recommendation. I don't think however that I would want to create a PR as part of a build pipeline.

I'm pretty sure that I'll be able to create a PR using one of the nuget packages listed here: https://docs.microsoft.com/en-us/azure/devops/integrate/concepts/dotnet-client-libraries?view=vsts

My vote is for option 2 as well. I do prefer to wait a little bit for it more than use the option 1.

I've created the following simple Powershell script a few months ago for creating PRs with VSTS. It worked very well for some build automations we did in a project. The idea was to create automatically some PRs that developers tends to forget, guiding them through GitFlow. We forced the creation of PRs from release or hotfix branches to master branch. But, this is another history.

Some updates will be necessary - like API version, for sample. And it was created to run under a build definition on VSTS (see the environment variables used), so you'll need to adapt it to your implementation.

`param ( [string]$targetBranch )

$sourceBranch = ($env:BUILD_SOURCEBRANCH).Replace("refs/heads/", "") $headers = @{Authorization = "Bearer $env:SYSTEM_ACCESSTOKEN"} $url = "$($env:SYSTEM_TEAMFOUNDATIONCOLLECTIONURI)$($env:SYSTEM_TEAMPROJECT)/_apis/git/repositories/$($env:BUILD_REPOSITORY_NAME)/pullRequests?api-version=4.1-preview" $body = ConvertTo-Json @{sourceRefName="$env:BUILD_SOURCEBRANCH";targetRefName="refs/heads/$targetBranch";title="[$env:BUILD_BUILDNUMBER] $env:BUILD_DEFINITIONNAME requested a merge from '$sourceBranch' to '$targetBranch'";description="$env:BUILD_BUILDNUMBER"} $ctype = "application/json"

try { $response = Invoke-WebRequest -Uri $url -Method POST -UseBasicParsing -Headers $headers -Body $body -ContentType $ctype
if ( $response.StatusCode -ne 201 ) { Write-Host "##vso[task.logissue type=error]Failed to create Pull Request - $response.StatusCode $response.StatusDescription" Write-Host "##vso[task.complete result=Failed]" return 1 }

Write-Host "Pull Request created for branch $targetBranch" return 0 } catch { Write-Host "##vso[task.logissue type=error]Failed to create Pull Request - $_.Exception.Message" Write-Host "##vso[task.complete result=Failed]" return 1 }`

[schnitty]

I agree, option 2 is the better solution. I've done some initial feasibility assessments and it looks like the client libraries are pretty useful and that pull requests can be created using them easily enough. I kind of see the work in two halves.

One half is to create some tests to ensure that we can achieve what we want to with the client libraries. Which is: For features/bugfix

• Create a PR to merging to develop and open web browser to the newly create PR
• Prevent feature/bugfix finish until PR is complete
• Delete local branch on finish

For release/hotfix

• Create a PR merging to develop and open web browser tab to newly created PR
• Create a PR merging to master and open web browser tab to newly created PR
• Prevent release/hotfix finish until both PRs are complete
• Create and push tags and delete local branch on finish

The second half then is: Incorporating this functionality into your VSIX extension and creating a suitable UI

Like most of us I'm pretty busy with other things so am not sure how much time I will have to devote to it. I'll see if I can get a start on part 1 and then I might publish so I can get some help on part 2 with some collaboration to make it quicker.

[ycamargo]

I agree, option 2 is the better solution. I've done some initial feasibility assessments and it looks like the client libraries are pretty useful and that pull requests can be created using them easily enough. I kind of see the work in two halves.

One half is to create some tests to ensure that we can achieve what we want to with the client libraries. Which is: For features/bugfix

• Create a PR to merging to develop and open web browser to the newly create PR
• Prevent feature/bugfix finish until PR is complete
• Delete local branch on finish

For release/hotfix

• Create a PR merging to develop and open web browser tab to newly created PR
• Create a PR merging to master and open web browser tab to newly created PR
• Prevent release/hotfix finish until both PRs are complete
• Create and push tags and delete local branch on finish

The second half then is: Incorporating this functionality into your VSIX extension and creating a suitable UI

Like most of us I'm pretty busy with other things so am not sure how much time I will have to devote to it. I'll see if I can get a start on part 1 and then I might publish so I can get some help on part 2 with some collaboration to make it quicker.

I think your strategy is ok. I'll try to find some time to collaborate on part 2. I'm not sure it's a good idea to open the browser. In general, people that will create the PR is different from people that will approve it. And, for simplicity, I would use the "finish" command instead of creating some sort of control to prevent it until PRs are completed. But I would include a push upfront in order to prevent loss of changes.

For release/hotfix I would not create the 2 PRs. I understand that a lot of teams work in that way but I do prefer to tag first, merge into master and, then, merge master to develop. In that way, the develop branch will always be one commit ahead of master. I think the better way should be:

For features/bugfix branches:

1. Push branch to remote repo;
2. Create a PR to merging to develop (we can just give the link to newly created PR);
3. Delete local branch and finish;

For release/hotfix branches:

1. Tag local branch and push it to remote repo;
2. Create a PR to merging to master (optionally, also create a PR to merge master into develop);
3. Delete local branch and finish;

[schnitty]

@ycamargo Thanks for your input and feedback.

I think that your idea for finishing release/hotfix branches has merit (Single PR to master then merge to develop instead of PR to develop and PR to master). It makes the process simpler and there often isn't a lot of point in having another PR for the same changes but to a different branch.

I think a lot of teams who are following this modified giflow with PR process (myself included) have done it with 2 PRs because their are two merges.

There is some benefit however for seperate PRs where you have different branch policies on your develop branch and master branches. I've had a scenario for example where a QA team were approvers on the master branch

I'd be interested to see what others think.

I don't like the idea of creating a PR and then finishing the feature and deleting the local branch. The feature isn't finished in my view until the PR is approved and completed. I like to keep my local branch of the feature so I can push updates to the published branch based on feedback I receive in the PR. Only once the PR is complete do I then delete my local branch. I understand that what you have suggested would be easier to implement but I don't think it follows how most people would like to work. It would be strange to have to pull and track a feature branch that you had finished to make changes to it in response to suggestions in a PR. You would then be doing this outside of the normal gitflow process. Also, our feature finish process would have to detect that a PR already exists when the person goes to re-finish their feature as we would have made creating a PR part of this finishing process. So I'm not sure it would be easier to implement in the end.

I think it is important to get these questions right for the intended audience of this tool. I see the audience (could be wrong) as predominately .NET developers who maybe aren't that familiar yet with git and gitflow. They like the familiarity of the Visual Studio GUI and like the tool support of preventing mistakes while they are learning.

This makes me want to lean towards a solution that makes the most sense to that audience.

[ycamargo]

Regarding the 2 PRs, in a recent project we've implemented policies for master and develop branches. We used to create 1 PR (release -> master) and after that PR got approved and completed we had to create another PR (master -> develop). To enforce the creation of this second PR, we created a build definition triggered by any commits on master that was responsible for:

1. version pump (based on some criteria, but mostly on last tag version);
2. tag master with new version;
3. automatically create the PR (master -> develop);

So, when the merge to master was completed, we automatically tag and create a PR to merge this to develop. This is related with but is beyond the scope of what we are trying to do here.

I think this could be a configurable behavior of our solution:

1. on "release/hotfix finish" command, just create only 1 PR to master or create 2 PRs (to master & to develop).
2. on "feature/bugfix finish" command, just create 1 PR to develop.

Regarding the deletion of local branch on finish or waiting for the PR approval & completion, I confess that I'm not sure. I agree with you that doesn't make much sense to delete something that is not completed but I still think it could be much easier to develop in that way. The only downside I can see could also be easily solved by instructing developers to (re)clone remote branches if they need to fix or change something that was appointed by PR reviewers. And when you need to (re)finish is easy to check if there are any other pending PR by retrieving the list of active PRs of the same branch (https://docs.microsoft.com/pt-br/rest/api/azure/devops/git/pull%20requests/get%20pull%20requests?view=azure-devops-rest-4.1). Also, I'm not 100% sure, but I think if you try to create a new PR with another one pending (for the same source and target branches) the API will return an error.

[schnitty]

There is a chance that I'll be able to get this into a sprint for work that I am doing for a client because they have a large number of staff who would benefit from it. If that happens then I will have suitable time to work on it to get it done. I'll keep you posted.

[yannduran]

I probably won't need to use this new functionality, but I'd just like to ask that the workflow for people who don't use pull requests not be overly modified (other than maybe some extra checkboxes etc), or made more difficult or convoluted.

I just ask whoever's implementing this that they keep that in mind. :-)

[schnitty]

It turned out that I didn't get time to work on this for my client, too many other higher priorities. It looks like v1.11 of GitFlow includes some options to push branches on feature finish that may make working with a PR flow much easier. https://github.com/petervanderdoes/gitflow-avh/releases/tag/1.11.0