Forbid pull requests from the dev branch?

[YanzhaoW]

As discussed in the R3B analysis meeting this week, a pull request should not be made from the dev branch of the forked repo. This concerns two PRs currently: #842 and #832.

So my proposal would be to reject any pull request made from the dev and master branch.

Please share your opinion if you don't agree with this restriction.

[jose-luis-rs]

Maybe you can explain what is the problem if the dev branch of the forked repo is synchronized with the main one (rebase) before the PR. According to my understanding it does not break the linearity, or at least, I have never seen the problem.

Of course, the recommendation is to use other branches for the PRs ...

[jose-luis-rs]

I forgot to say that we should avoid the use of the option "merge" because this breaks the linearity of our repository, all the users must use always the command "rebase". This is very important to avoid the propagation of conflicts along all the forked repos.

[klenze]

IMHO, every dev branch everywhere dev should track R3BRoot main repo dev.

Changes belong in a feature branch, which can then pushed to the submitters git repo.

(Personally, I prefer git cloning from the R3BRoot main repo and then adding my own github remote. I know that it is generally taught the other way round in our collaboration.)

Making PRs from the submitters dev branch will obviously fail if they have more than one PR at the same time. If someone makes has two open PRs wanting to merge from their dev branch, summarily closing both of them seems like a reasonable fix.

@YanzhaoW: while I agree that PRs from some dev branch are messy, and there might be a correlation between that and the quality of the PR, I don't think it is feasible (or desirable) to use that as a filter, if that was your intention. Anyone able to create a PR will also be able branch and then create a PR for their branch within one hour.

Personally, I would prefer to reject PRs for their content instead of rejecting them for formal stuff like this or "you did not capitalize califa in your commit message" or whatever.

Pointing out "Your PR has yourrepo:dev as a source, please try to create a feature branch next time" is fine. Wasting their time by having them redo their PR (in addition to having to (eventually) fix their local dev branch, which has now diverged from mainrepo:dev) does not seem Pareto-optimal.

To state it differently: most experienced devs familiar with git will make PRs from feature branches. Unfortunately, you can not simply turn a contributor into an experienced developer by making them create feature branches for their PRs. See also: Goodhart's law.

Again, if there is something more fundamentally bad about PRs from dev, please tell me.

[YanzhaoW]

From this guidelines, the first is stated with:

• Make A Branch Please create a separate branch for each issue that you're working on. Do not make changes to the default branch (e.g. master, develop) of your fork.

There should be some legitimate reasons behind this.

Personally I think if someone makes a pull request from the dev/master branch (,in other words, makes dev branch as the feature branch), it could cause serious problems when he tries to rebase the dev branch to the dev branch of the remote repository. And of course, if he does this, he can have only one feature branch. There could be some other important reasons though.

Anyway, if we don't want to fail the CI test deliberately because of the pull request made from a dev branch, maybe we could add another check list item in our pull request template? Something like:

• [ ] Followed the pull request guidlines

[hapol]

Let me remind that we should follow the git workflow (see Developers part in the workflow description): https://github.com/AnarManafov/GitWorkflow/blob/master/GitWorkflow.markdown where explicitly requires the creation of a feature branch for any new modification.

[jose-luis-rs]

Ok, I will add this point to the PR template.