#907 Support multiple workflows
Closed: Won't Fix by wombelix. Opened by firstyear.

Pagure is designed around the Github style Fork / Pull Request model. Generally the outline of this is:

  • Someone wants to write a new feature, so they fork the repository to their own.
  • Do the work
  • commit to their fork
  • create a pull request
  • project merges.

However, this model gets tricky when other things happen.

  • Someone wants to write a new feature, so they fork the repository to their own.
  • Do the work
  • commit to their fork
  • create a pull request
  • Pull request won't merge as changes have happen. Rebase and new commit
  • review asks for more changes to be accepted
  • Another commit happens
  • project merges.

So here, there are questions about squashing / force pushing the changes to make master of the project "always clean". As well, it adds some other complexities.

Many existing projects on fedorahosted.org are on trac, and use the patch / rebase workflow.

This is:

  • pull from master
  • create local branch
  • do work
  • git format-patch > send to trac
  • review
  • make more changes, squash to single commit, format-patch > trac for review
  • patch is acked, git commit --amend to sign off from reviewer.
  • rebase master to make sure it applies
  • checkout master
  • git cherry-pick commit-id

This workflow also has it's own issues. Where it does well is squashing is not as intrusive for branches.

Projects would like to be able to choose their workflow. Some people prefer Pull Request. Some people prefer Patch Rebase.

It would be good for pagure to support both. This could be achieved by:

  • setting on the repository to allowing attachment of files to issues.
  • If the attachment is a patch, display it in the same way as a commit.
  • Setting on the repository to enable / disable forking of the repository.

A combination of these settings would allow / disallow

  • Pull Request
  • Patch Rebase
  • Both at the same time!

Thanks for your ideas, I think I agree with most of them and there are already a few solutions to allow supporting both approaches.

The PR/Merge approach as you pointed out is pretty much already there.

For the Patch/Sign-Off approach, the idea has been so far:

  • Open a PR
  • Review the changes, update them as needed, ask the reviewee to squash them if desired... as a regular PR
  • Once the changes are satisfactory, download the patch (click on the 'Patch' button in the PR page or simple wget https://pagure.io/project/pull-request/id.patch)
  • Add sign-off-by, add a line 'Fixes https://pagure.io/project/pull-request/id), squash if it wasn't there, rebase if needed, commit, push

Adding the 'Fixes...' line allows closing the PR as merged when pushing the commit (if the pagure hook/plugin is enabled).

So I agree it is not exactly what you described, it is a bit of a mixed-model but I do think it gets close to what you describe.

Do you think this could work for you?

I think that I would need to try it to make a completely fair assement. My concern would be that we still need the PR, and to update the PR, we still need to force push a squashed set of commits for "review". I would like to avoid the --force flag at all possible.

So for now, I think I would like to try your suggestion, but I think that perhaps there is room to improve both workflows in subtle ways.

Thanks for your time!

EDIT: What about workflows that want to block one or the other? Some projects may want to prevent PR's completely? Some others may want to prevent patch submissions?

Some projects may want to prevent PR's completely?

There is already a setting for this :)

Some others may want to prevent patch submissions?

I am not sure how we could do that, it would mean disabling attaching a file to issues which I think can be a good feature if you need to share a screenshot or so.

I would like to avoid the --force flag at all possible.

I can understand that totally, if you are the one doing the squashing at the end of the review, it should work.

One advantage (for your approach as well) of the PR page is that you will know before hand if the PR is conflicting or not.

The last update was 6 years ago, no further requests, updates or actionable tasks since then, I'm going to close this issue for now to reduce our backlog.

Metadata Update from @wombelix:
- Issue close_status updated to: Won't Fix
- Issue status updated to: Closed (was: Open)

Metadata