[github] default PR destination
hi all, any opinion to change the default branch from "master" to "develop"? with the boost branching model all contributions via github's pull requests apparently have to go to "develop" rather than going directly into "master", so this should be the default destination for pull requests. github allows to set the default branch [1] ... not sure if this has any other side effects, but i'd vote for changing the default to "develop". thoughts? tim [1] https://help.github.com/articles/setting-the-default-branch
On 01/04/2014 05:51 AM, Tim Blechmann wrote:
hi all,
any opinion to change the default branch from "master" to "develop"?
with the boost branching model all contributions via github's pull requests apparently have to go to "develop" rather than going directly into "master", so this should be the default destination for pull requests. github allows to set the default branch [1] ... not sure if this has any other side effects, but i'd vote for changing the default to "develop".
thoughts? tim [1] https://help.github.com/articles/setting-the-default-branch
tl;dr: leave it up to the individual module developer based on the answer to this question: Who's submitting pull requests, and for what purpose? FWIW, I have submitted four PRs to Boost in the past two weeks. One to the superproject, one each in three modules. Two are trivial fixes to showstopper bugs; one is a documentation fix relevant only to new developers; and one aggregates fixes for a couple bugs (one showstopper) and enhances functionality. All address issues visible in Boost 1.55. So I think I'm representative of one class of people who will use pull requests. Here's my use case: I use open source, preferring package versions supplied by the OS vendor (e.g. Ubuntu). When I need something more recent, I install from a versioned release tarfile. When I discover problems, I try to locate a git repository for the project and locate an existing patch in a subsequent release or development branch. For my local installation I branch off the commit corresponding to the release I'm using, and either back-port the existing patch or fix the bug myself. If there isn't already a solution, I open a bug report and attach my patch to it; for github, that's creating an issue submitting a pull request referencing it. I'm acting as a Boost user (not developer), who is able to provide a proposed solution along with the problem report. Now: Of the four pull requests I submitted, three were based off develop, and one off master, but that's not representative because I assumed develop was more stable and decoupled than it is. The last one (regex) I did off master, and I expect that most future ones will also be off master. Only when I'm acting as a Boost contributor (e.g. the string_ref enhancements for utility) would I create a workspace that checks out all the develop branches and make the extra effort to test what I've done against that as opposed to the stable system for which I need the change. So most of my future pull requests will be based off tags on the master branch. For modules that follow git-flow, those tags will reference commits that are not on develop, so setting the default branch to develop will create horrible "your branch is 350 commits ahead and 452 commits behind base" diagnostics. If modules ultimately push to their master branch only when the superproject releases, then master is the best default basis for pull requests in my situation. If after things settle modules continue to push to master multiple times throughout a release cycle, I'll always have to reset the base to the release version, but they would still at least be on the same branch as the default. Switching to develop is not appropriate for my situation, which is user-oriented. So move on to developer-oriented use cases: I expect module developers will not be making pull requests for their own modules, unless the module-level process imposes a strict peer-review expectation. So there is no need to switch the default to develop in support of the developers of that module. It may be that developers of one module may wish to submit a change to another, e.g. to resolve interoperability issues. If that happens, develop is the correct base. I have no basis for a guess as to likelihood. Finally, when you submit a pull request github clearly shows those 356 commits ahead and 452 commits behind, so if you understand enough about git/github to fork, branch, and submit a pull request you probably can figure out how to change the base so it's correct. Summarizing: I see no need for a Boost-wide policy, and no need for individual modules to change anything until there's experience justifying the change. Peter
On 01/05/2014 08:50 AM, Peter Dimov wrote:
Peter A. Bigot wrote:
The last one (regex) I did off master, and I expect that most future ones will also be off master.
What is a developer supposed to do with a pull request against master?
Pull the branch to a local repository, merge it into develop, test it, and reject or approve it. Normal git practice: see https://help.github.com/articles/merging-a-pull-request under "Merging locally". My point: As a user, I need a working stable tool, so my time goes toward creating that. I'll donate the extra time to submit any solution I find back to the project, but I'm not going to donate the hours it'd take create/update a developer-oriented workspace and rebuild everything just to see whether my patch also works for the developer, who's already set up for that sort of thing. If that's going to be a problem, I'll be happy to skip the give-back-to-Boost step. (for tim: same answer; pull request is never directly merged to master; any test farm is not inherently circumvented; etc.) Peter
On 01/05/2014 09:34 AM, Peter A. Bigot wrote:
On 01/05/2014 08:50 AM, Peter Dimov wrote:
Peter A. Bigot wrote:
The last one (regex) I did off master, and I expect that most future ones will also be off master.
What is a developer supposed to do with a pull request against master?
Pull the branch to a local repository, merge it into develop, test it, and reject or approve it. Normal git practice: see https://help.github.com/articles/merging-a-pull-request under "Merging locally".
If master and develop do not share commits (as I believe they will not under git-flow): branch off develop, merge the pull request to the branch, then rebase it on develop, preserving the contributor information but also making it clean. Again, this is something that is trivial for the developer to do and more than should be expected of contributing users. This sort of thing is exactly what git-rebase is for. Peter
On Sun, Jan 5, 2014 at 8:44 AM, Peter A. Bigot
On 01/05/2014 09:34 AM, Peter A. Bigot wrote:
On 01/05/2014 08:50 AM, Peter Dimov wrote:
Peter A. Bigot wrote:
The last one (regex) I did off master, and I expect that most future ones will also be off master.
What is a developer supposed to do with a pull request against master?
Pull the branch to a local repository, merge it into develop, test it, and reject or approve it. Normal git practice: see https://help.github.com/articles/merging-a-pull-request under "Merging locally".
If master and develop do not share commits (as I believe they will not under git-flow):
I noticed that there are some submodules where develop is based on master, i.e. if you run "git branch --contains master", both master and develop are output. Most of them, like you say, show master and develop to be distinct. Won't that be a problem when trying to merge from develop to master? Just for grins, I took the accumulator submodule in which master and develop don't share commits and tried to locally rebase develop onto master. Unfortunately, the rebase failed early, but the few files I looked at have the same change trying to be applied on both sides, i.e. the lines between >>>> and ==== were the same as the lines between ==== and <<<<<. Any idea why that's the case? It looks like develop might be able to be "reconnected" to master (at least in accumulator), but it would take someone who understands the code and history of changes. Michael
If I were a boost library owner, I would be quite annoyed if someone submitted a pull request against master, even though regulations stipulate that all modifications must go through develop in order to cycle through tests. This can create headache for the maintainer, whose working branch may be heavily modified. In my experience maintaining git repos, I would expect contributors to be up to par with the develop branch. They need to know the changes, else they may not even be contributing anything useful. If that is too much work for someone, then that person's code will probably not get merged. Sure, they can make an issue / bug report or submit a pull request, but I'll at most cherry pick it to dev and most likely find a better solution myself. Because of the wide scope in which boost is used, it makes sense for all pull requests to go through develop, and because of that it also makes sense that the default PR branch is develop. cheers, Rich
On 01/08/2014 03:27 AM, Cox, Michael wrote:
On Sun, Jan 5, 2014 at 8:44 AM, Peter A. Bigot
wrote: On 01/05/2014 09:34 AM, Peter A. Bigot wrote:
On 01/05/2014 08:50 AM, Peter Dimov wrote:
Peter A. Bigot wrote:
The last one (regex) I did off master, and I expect that most future ones will also be off master.
What is a developer supposed to do with a pull request against master?
Pull the branch to a local repository, merge it into develop, test it, and reject or approve it. Normal git practice: see https://help.github.com/articles/merging-a-pull-request under "Merging locally".
If master and develop do not share commits (as I believe they will not under git-flow):
I noticed that there are some submodules where develop is based on master, i.e. if you run "git branch --contains master", both master and develop are output. Most of them, like you say, show master and develop to be distinct. Won't that be a problem when trying to merge from develop to master?
I am not sure how much it matters, if any what branches is listed by "git branch --contains master". Apply/commit, merge from other remote tracking branches, or pull contributions such as GitHub PRs to develop as they where applied to trunk in SVN. It make a lot of sense for all maintainers to maintain public devlop head as the point they expect every contributor to base their contribution upon. Every other long running or deferred work not expected to go into next release should be put into feature branches. I am not familiar with pulling GitHub PR, but I expect it to be possible to merge localy and commit, then before pushing to GitHub make the assessment whether it should be put in a feature branch or stay in develop, if it need to go in a feature branch you may make the feature branch at the new commit and reset develop head to previous commit. git checkout -b feature/PR-id-descriprion git checkout develop git reset HEAD^ this is very simple to do, understand, and verify correctness of what you do using some GUIs, e.g. using "gitk" tool that come with git distribution. In gitk right click the commits description to select "create new branch here" and "reset develop to this" commands. You see the result immediately in the commit tree and know it is what you want before pushing to GitHub. Note, if you want gitk to show more than current branch, use gitk --all, or create/edit view in menu. Don't underestimate gitk based on looks, it is actually very elegant and powerful to use if you try it a bit. It is up to the maintainer to merge submodule releases only into master. Thus it make no sense in general to base contributions on master as develop may contain a lot of stuff for next release. I would vote strongly for always using develop head as base for contributions.
Just for grins, I took the accumulator submodule in which master and develop don't share commits and tried to locally rebase develop onto master. Unfortunately, the rebase failed early, but the few files I looked at have the same change trying to be applied on both sides, i.e. the lines between >>>> and ==== were the same as the lines between ==== and <<<<<. Any idea why that's the case?
no initial merge in git has been performed yet for accumulators: https://svn.boost.org/trac/boost/wiki/PostCvtMergePoint so I don't really know what to expect. And I don't know if we should care much as it is not fair to git to expect much specific when trying to blindly merge to disjoint trees.
It looks like develop might be able to be "reconnected" to master (at least in accumulator), but it would take someone who understands the code and history of changes.
yes, this needs to be done with some special care as you indicate to help Git perform sensible automatic of merge later. -- Bjørn
On 01/05/2014 10:06 AM, tim wrote:
(for tim: same answer; pull request is never directly merged to master; any test farm is not inherently circumvented; etc.) so if you agree that your PR should not go into master, why do you request exactly that?
I think there's a misunderstanding of what "pull request" means. I'm not requesting that the changes go (directly) into master. I'm requesting (or suggesting) that the changes go into the project. It happens that I'm providing those changes relative to the current master because that's what I need and have available. But if they go through intervening branches and quality control before they end up back there, that's expected and something the maintainer should be able to easily accommodate using git. Peter
Now: Of the four pull requests I submitted, three were based off develop, and one off master, but that's not representative because I assumed develop was more stable and decoupled than it is. The last one (regex) I did off master, and I expect that most future ones will also be off master. Only when I'm acting as a Boost contributor (e.g. the string_ref enhancements for utility) would I create a workspace that checks out all the develop branches and make the extra effort to test what I've done against that as opposed to the stable system for which I need the change.
So most of my future pull requests will be based off tags on the master branch.
from my understanding the svn policy was that changes *have* to go to trunk before they can be merged to the release branch. reason changes should not be merged to master(==release) before tests on develop(==trunk) have cycled. even if a contribution is based on "master" it should go to "develop" first. now if a contributor issues a PR to the master branch, the PR circumvents the testfarm, which imho is a bad thing and should definitely not be the default. all your arguments about checking the git history are fine, but simply don't apply in the case that master==release. tim
On 1/5/2014 9:21 AM, Peter A. Bigot wrote:
On 01/04/2014 05:51 AM, Tim Blechmann wrote:
hi all,
any opinion to change the default branch from "master" to "develop"?
with the boost branching model all contributions via github's pull requests apparently have to go to "develop" rather than going directly into "master", so this should be the default destination for pull requests. github allows to set the default branch [1] ... not sure if this has any other side effects, but i'd vote for changing the default to "develop".
thoughts? tim [1] https://help.github.com/articles/setting-the-default-branch
tl;dr: leave it up to the individual module developer based on the answer to this question: Who's submitting pull requests, and for what purpose?
FWIW, I have submitted four PRs to Boost in the past two weeks. One to the superproject, one each in three modules. Two are trivial fixes to showstopper bugs; one is a documentation fix relevant only to new developers; and one aggregates fixes for a couple bugs (one showstopper) and enhances functionality. All address issues visible in Boost 1.55. So I think I'm representative of one class of people who will use pull requests.
Here's my use case: I use open source, preferring package versions supplied by the OS vendor (e.g. Ubuntu). When I need something more recent, I install from a versioned release tarfile. When I discover problems, I try to locate a git repository for the project and locate an existing patch in a subsequent release or development branch. For my local installation I branch off the commit corresponding to the release I'm using, and either back-port the existing patch or fix the bug myself. If there isn't already a solution, I open a bug report and attach my patch to it; for github, that's creating an issue submitting a pull request referencing it. I'm acting as a Boost user (not developer), who is able to provide a proposed solution along with the problem report.
Now: Of the four pull requests I submitted, three were based off develop, and one off master, but that's not representative because I assumed develop was more stable and decoupled than it is. The last one (regex) I did off master, and I expect that most future ones will also be off master. Only when I'm acting as a Boost contributor (e.g. the string_ref enhancements for utility) would I create a workspace that checks out all the develop branches and make the extra effort to test what I've done against that as opposed to the stable system for which I need the change.
So most of my future pull requests will be based off tags on the master branch. For modules that follow git-flow, those tags will reference commits that are not on develop, so setting the default branch to develop will create horrible "your branch is 350 commits ahead and 452 commits behind base" diagnostics.
If modules ultimately push to their master branch only when the superproject releases, then master is the best default basis for pull requests in my situation. If after things settle modules continue to push to master multiple times throughout a release cycle, I'll always have to reset the base to the release version, but they would still at least be on the same branch as the default.
Switching to develop is not appropriate for my situation, which is user-oriented. So move on to developer-oriented use cases:
I expect module developers will not be making pull requests for their own modules, unless the module-level process imposes a strict peer-review expectation. So there is no need to switch the default to develop in support of the developers of that module.
It may be that developers of one module may wish to submit a change to another, e.g. to resolve interoperability issues. If that happens, develop is the correct base. I have no basis for a guess as to likelihood.
Finally, when you submit a pull request github clearly shows those 356 commits ahead and 452 commits behind, so if you understand enough about git/github to fork, branch, and submit a pull request you probably can figure out how to change the base so it's correct.
Summarizing: I see no need for a Boost-wide policy, and no need for individual modules to change anything until there's experience justifying the change.
Normally library developers will not merge changes from 'develop' to 'master' until regression tests cycle through for a reasonable period of time ( normally about a week to a month ) showing that any changes made in 'develop' are working properly. The fact that a library has many changes made in 'develop' over a long period of time which has not been merged into 'master' does not change the general fact that this is not the way the vast majority of library developers work.
Am 04.01.2014 12:51 schrieb Tim Blechmann:
any opinion to change the default branch from "master" to "develop"?
If you're sharing the same point of view as GitHub (and me, btw) that the 'default' branch is the primary focus of all contributions (that includes the maintainers), then it's just natural to set 'default' = 'develop'. It follows the Boost development model, it's explicit about intent, and it doesn't confuse contributors with their GitHub interactions. Ciao Dani
On 01/05/2014 11:03 AM, Daniela Engert wrote:
Am 04.01.2014 12:51 schrieb Tim Blechmann:
any opinion to change the default branch from "master" to "develop"?
If you're sharing the same point of view as GitHub (and me, btw) that the 'default' branch is the primary focus of all contributions (that includes the maintainers), then it's just natural to set 'default' = 'develop'. It follows the Boost development model, it's explicit about intent, and it doesn't confuse contributors with their GitHub interactions.
OK, I give. I agree that new features and major changes, whether from maintainers or contributing developers, should be submitted in pull requests based on develop. Nothing I wrote was intended to imply otherwise. Boost is using GitHub, and subtle differences in roles just confuse people, so let's stick with the Boost developer perspective. Then it should be Boost policy that all boost modules set the default branch to develop. This means anybody cloning the repository will have the develop branch checked out by default, including the super-project if the policy applies there as well. My user-level fix-problem-in-stable-release-and-provide-patch use case stands, and experience will show whether maintainers are willing to accept simple bug-fix pull requests that are based on master. I hope that such patches will not be summarily rejected solely because they require an little extra effort from the maintainer, since the contributor's already put in the effort to isolate the issue and propose a solution. Peter
On Sat, Jan 4, 2014 at 6:51 AM, Tim Blechmann
hi all,
any opinion to change the default branch from "master" to "develop"?
with the boost branching model all contributions via github's pull requests apparently have to go to "develop" rather than going directly into "master", so this should be the default destination for pull requests. github allows to set the default branch [1] ... not sure if this has any other side effects, but i'd vote for changing the default to "develop".
thoughts?
This is a very interesting thread, but I just don't have enough experience with pull requests to form an opinion yet. Worse yet, many other Boost contributors don't have pull request experience either. I'm starting to get pull request for libraries I maintain, and will process them ASAP to develop experience. It looks like other libraries are also starting to get pull requests. The opinions of people who are relatively new to this list are also important, but since this question doesn't seem to require immediate action I think it is just going to have to bake a awhile longer. Thanks, --Beman
participants (10)
-
Beman Dawes
-
Bjørn Roald
-
Cox, Michael
-
Daniela Engert
-
Edward Diener
-
Peter A. Bigot
-
Peter Dimov
-
Rich E
-
tim
-
Tim Blechmann