Re: Stan github member

[Bob Carpenter]

[back on stan-dev]

On 5/14/13 5:12 PM, Marcus Brubaker wrote:
> Hey guys,
>
> It looks like I got removed from the Stan project on github. I'm
> guessing this was an accident, but I just wanted to check to see what
> was going on.

After discussion during the meeting (which everyone's welcome
to join, by the way), we turned off push privileges for everyone
to enforce the pull-based workflow.

We should have waited until we had the process better sorted.
I thought it was going to be simpler. Daniel had to take off
before getting it documented with pointers to how to do the
various steps. Hopefully it'll be sorted out tomorrow (Thursday).

I believe Daniel and I still have push privileges intended to be
used for C++/core Stan and Jiqiang and Ben still have push privileges
intended to be used only for RStan. I'm worried this will be
too much of a bottleneck, so we can extend review/push privileges
back to everyone who had push permission before. This wasn't intended
to be a rebuke of anyone's code --- I actually thought we could turn
everyone's push priviges off and do everything through pull, but it
looks like you need push privileges to be able to execute someone
else's pull request.

What I don't want to do is have people push things directly.
Hopefully simple changes will be simple pull requests and
get handled quickly. I wrote to Marco, who has the outstanding
pull requests, and told him we needed (a) unit tests for auto-diff
and (b) were waiting until we had the process sorted.

I think if we're going to do this then EVERYTHING should get
reviewed. I don't want it to be a judgment call on the part
of the person doing the pushing, or I think it'll quickly devolve
to the old system.

Hopefully the simple changes will be simple and fast.

I have a pending push that I'm still trying to figure out how to
formulate into a pull request. Looks more complex than I anticipated.
The problem I have with git is that every answer to a question
about git on a blog post or on stack overflow seems to require a
Ph.D. in git to understand. It's worse than unix man pages.

So we'll see how it goes. We can always change the process
again at a later date if this experiment is a miserable failure.

- Bob

[Marcus Brubaker]

Could you remind me when the meetings are and send me info on how to
join them remotely? I've never been able to figure that one out.

I suspect that people will need push privileges to merge pull
requests. In particular, any merge which isn't clean I believe has to
be done manually. While this isn't an issue for any branch which is
tracking master, if an outside contributor sends a pull request which
requires manual merging, then someone has to do it locally. (See
https://help.github.com/articles/merging-a-pull-request ) Ideally
whoever submits it would track master, but one-off contributors may
not. (I suppose they could merge locally and then create a separate
pull request, but that seems tedious.)

Cheers,
Marcus

> --
> You received this message because you are subscribed to the Google Groups
> "stan development mailing list" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to stan-dev+u...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Ben Goodrich]

On Tuesday, May 14, 2013 7:48:21 PM UTC-4, Bob Carpenter wrote:

After discussion during the meeting (which everyone's welcome
to join, by the way), we turned off push privileges for everyone
to enforce the pull-based workflow.

I think it also made it the case that I can't close / tag issues from GitHub anymore.

But I don't think our problems are that strongly related to not using pull requests. We just have poor workflow.

Ben

[Bob Carpenter]

On 5/14/13 7:58 PM, Marcus Brubaker wrote:
> Could you remind me when the meetings are and send me info on how to
> join them remotely? I've never been able to figure that one out.

12:30 EDT Tuesdays via google+ hangout. If you have
a google+ login, send it to Daniel who does the invites.
We should probably have a master list if that's possible
so we can deal with it when Daniel's not around.

> I suspect that people will need push privileges to merge pull
> requests. In particular, any merge which isn't clean I believe has to
> be done manually. While this isn't an issue for any branch which is
> tracking master, if an outside contributor sends a pull request which
> requires manual merging, then someone has to do it locally. (See
> https://help.github.com/articles/merging-a-pull-request ) Ideally
> whoever submits it would track master, but one-off contributors may
> not. (I suppose they could merge locally and then create a separate
> pull request, but that seems tedious.)

I voted for going to pull requests before realizing how hard this
was going to be to put into practice with enforcement. I think you're
right that we're just going to have to open it back up to all the
core members and then just trust everyone to be on their best behavior
in working through pull requests.

Suggestions on the process we should use are more than welcome.

- Bob

[Bob Carpenter]

Could you elaborate on what you think a better workflow would be?
Ideally in terms I can understand that don't involve page after
page of Git diagrams? If not, I can try to get through something
more involved over the next few days. But I really don't understand
all of these pages of git instructions.

Is there something shorter than a book I can read about the basics?
I don't want managing git/GitHub to take over my life.

- Bob

[Marcus Brubaker]

I suspect his point is that we all just need to be better about using
things like branches and pull requests which is almost certainly true.

Though to be honest, I think things were generally working OK, with
the exception of the recent refactor which landed heavily. That said,
a change that large (effectively rewriting an entire subsystem) is
bound to kick up a lot of dust and I think much of it was inevitable.
Plus, I doubt any amount of pull request code review would have caught
the issues with NUTS.

Cheers,
Marcus

[Bob Carpenter]

It's not just the big rewrite of Michael's.

Part of the problem with the big rewrite was that Michael
misunderstood what Daniel and I meant by "refactor". For
us, it meant changing the code, but not changing the behavior.
For Michael, it meant fixing everything so it was conceptually
cleaner and "right" in some sense.

The real problem, though, was lack of testing for the models.

You're right that the subtle bugs wouldn't have been caught
by code review, but hopefully they'd have been caught by
better functional tests.

But we've had lots of other code that's broken the build get
pushed.

I want to move to a system where we try to keep the master
branch in a clean, releasable state at all time.

For the big rewrite, that would've meant keeping it in a branch
until it was fully baked. Again, that was a misunderstanding
about what "done" meant among me, Daniel and Michael.

- Bob

[Michael Betancourt]

> It's not just the big rewrite of Michael's.
>
> Part of the problem with the big rewrite was that Michael
> misunderstood what Daniel and I meant by "refactor". For
> us, it meant changing the code, but not changing the behavior.
> For Michael, it meant fixing everything so it was conceptually
> cleaner and "right" in some sense.
>
> The real problem, though, was lack of testing for the models.
>
> You're right that the subtle bugs wouldn't have been caught
> by code review, but hopefully they'd have been caught by
> better functional tests.
>
> But we've had lots of other code that's broken the build get
> pushed.
>
> I want to move to a system where we try to keep the master
> branch in a clean, releasable state at all time.
>
> For the big rewrite, that would've meant keeping it in a branch
> until it was fully baked. Again, that was a misunderstanding
> about what "done" meant among me, Daniel and Michael.

To be fair, I did keep the refactor in it's own branch, we just merged prematurely because we didn't have a pull request framework in place. Part of a pull request should be compiling and running unit-tests (most big orgs automate these tasks, but it's not too hard to do it by hand for now) and identifying what should be tested that has not yet been tested. With acid tests we'll be closer to testing overall model performance, too, and not just rigid behavior testing.

[Ben Goodrich]

On Tuesday, May 14, 2013 8:08:29 PM UTC-4, Bob Carpenter wrote:

> But I don't think our problems are that strongly related to not using pull requests. We just have poor workflow.

Could you elaborate on what you think a better workflow would be?
Ideally in terms I can understand that don't involve page after
page of Git diagrams?  If not, I can try to get through something
more involved over the next few days.  But I really don't understand
all of these pages of git instructions.

I think it is probably at least as important that we all standardize on a workflow as it is to debate the merits of the multiple workflows supported by git / github / gerrit / etc. But here are a bunch of things I think

-- If a project has good tests, then that mitigates a lot of problems. But:

---- Our test suite has grown so big that it is not feasible for everyone to run on every commit
---- There are still things that are not easily caught by the unit tests
---- There are some things that pass test-unit with whatever toolchain the author is using but not some other toolchain

To overcome this somewhat, I've emailed to see if we can push to a repository on Hotfoot that would run test-unit in about 2 minutes. That would make it feasible to test every commit and only use the Windows box that Jenkins is on for batches of commits that may be ready for master.

-- Our commits are not good.

---- The commit messages are often not that informative
---- The commits lump together things that should be separate commits
---- Separate commits are used for things that should be lumped together (like "made change, will test and document later")
---- Stuff gets committed that doesn't compile. Note that this is a "git commit" issue rather than a "git push" or "git pull" issue. In other words, this is bad:

Commit A: Add feature, fix bug, whatever
Commit B: Oops, made mistake that broke compilation, now fixed

If you push both of these commits, then (among other reasons) if there is a need to "git bisect" the tree to fix some subtle problem and the bisection lands on Commit B, then git can't tell whether Commit B is good or bad with respect to the subtle problem under investigation because it just terminates in a compiler error. This happened most recently with the different numerical results between g++ (!Windows) and mingw64; I couldn't get to the first commit that exacerbated the numerical differences because it was somewhere in a sequence of about 20 commits that didn't even compile.

The thing to do is either use the --amend flag to "git commit" when committing B to the history, which doesn't actually commit B to the history but instead changes Commit A so that it incorporates the change induced by Commit B and thus (for historical purposes) never breaks the build. Or if you discover that Commit A was faulty after already making some intermediate commits, you can use "git rebase -i" to squash Commit A and Commit B into a single commit (say Commit AB) before pushing anywhere (and thereby making the history essentially irrevocable). If we do that, then "git bisect" often locates the root of the problem quickly and essentially automatically.

-- Stuff is going into master way too soon

--- Where it should go depends on a project's workflow, but there is never a rush to go into master and master should always be usable. In other words, anyone should be able to clone the master branch on GitHub and get a working Stan or RStan so we have to be careful about when and what gets merged into master.
--- Branches are so cheap in git and so easy to work with in GitHub that nothing is lost by saying "I've pushed some stuff into branch foo. Can people test it / comment on it and we'll see about merging it into master later?" We had such a branch with the hmc_refactor thing, but it got merged into master before there was time to figure out all the issues that a change of that magnitude would raise.

-- I don't think pull requests are all that great for Stan in general

---- There are a qualified blessing for outside contributors that happen to have GitHub accounts
---- They are a nuisance if the pull request is just something like "I eliminated some already completed cruft off the TO-DO list"
---- Having someone else give approval is overkill if the pull request is something like "I added a test that passes". 99% of the time there is going to be no objection and the 1% of the time that the test is actually flawed is not worth the hassle of going through the pull request
---- If something merits a pull request, it probably merits a group discussion and testing with multiple toolchains, as opposed to one person approving the pull request. This is because Stan is complicated and most people don't know that much about certain parts of it. In other words, I would favor a process where bigger changes are discussed and tested, objections are resolved, and when there are no more objections, someone pushes it to master.

-- I've never used gitflow myself, but it is widely used and seems to help with discipline

---- There is the master branch and the development branch (among others)
---- New features get pushed to the development branch before they are pushed to the master branch. In other words, the development branch is somewhat like how we have been using the master branch thusfar
---- There are also hotfix branches that fix bugs but do not add new features. A hotfix branch is merged into both the development branch and the master branch.
---- Whenever the master branch changes, you (can) do a release

You probably want some degree of human review before something goes into development (at least if it touches stan/src) and definitely want human review before it goes into master. But you don't want too high of a hurdle for a push to some feature branch that maybe multiple people are working on before it gets pushed to the development branch.

Git is complicated, but it is now beating every other version control system for open source projects, including ones that are bigger or more complicated than Stan. Few projects move away from git and many are moving to it, which causes temporary headaches for people learning the ropes. But we've been using git for a long time now, and it shouldn't be beyond our capability to use it well.

Ben

[Bob Carpenter]

On 5/14/13 8:44 PM, Michael Betancourt wrote:
>
>> It's not just the big rewrite of Michael's.
>>
>> Part of the problem with the big rewrite was that Michael
>> misunderstood what Daniel and I meant by "refactor". For
>> us, it meant changing the code, but not changing the behavior.
>> For Michael, it meant fixing everything so it was conceptually
>> cleaner and "right" in some sense.
>>
>> The real problem, though, was lack of testing for the models.
>>
>> You're right that the subtle bugs wouldn't have been caught
>> by code review, but hopefully they'd have been caught by
>> better functional tests.
>>
>> But we've had lots of other code that's broken the build get
>> pushed.
>>
>> I want to move to a system where we try to keep the master
>> branch in a clean, releasable state at all time.
>>
>> For the big rewrite, that would've meant keeping it in a branch
>> until it was fully baked. Again, that was a misunderstanding
>> about what "done" meant among me, Daniel and Michael.
>
> To be fair, I did keep the refactor in it's own branch, we just merged prematurely because we didn't have a pull request framework in place.

Right! That's why I told Ben it was ironic that this happened
when we were trying to follow protocol because we knew it was
dangerous.

The problem is that we were all too eager to get it
into master, so it went in before it was done. Pull requests
wouldn't have helped at all here. What would've helped is
more tests.

Pull requests wouldn't have helped at all. Also, this is just
too much code to read in a diff, so it's really an unusual case.

> Part of a pull request should be compiling and running unit-tests (most big orgs automate these tasks, but it's not too hard to do it by hand for now) and identifying what should be tested that has not yet been tested.

Daniel's figuring out how to do that on Jenkins.

> With acid tests we'll be closer to testing overall model performance, too, and not just rigid behavior testing.

They're usually called "functional tests" or "regression tests". And yes,
this was the heart of the problem.

We should've had them in there from the beginning!

The judgment call's always been whether to try to add more
functionality or do more testing to make what we have more
solid. It's a tough call.

- Bob

[Bob Carpenter]

Thanks! That's the level of description I was
looking for (and what Marcus sent before). I particularly
like the git bisect argument. And compiling on hotfoot
would be great.

As to different platforms, we can set up multiple Jenkins-like
environments with a Windows, Mac, and Linux platform. It's
a lot of complexity, though. But if we're merging to master
less frequently, then perhaps they don't have to be run that
often.

In any case, we should go back to giving everyone
commit permission. Daniel?

- Bob

[Daniel Lee]

I think I've finally caught up on all the thoughts. Sorry about being unavailable the last few hours.

My opinions differ from most of what's being discussed. I'm first going to restore everyone's privledges for the time being, but I don't think this is the right thing to do. (Ben, what's your GitHub handle? Your commits aren't being picked up by GitHub as a user.)

Here is what I suggest:

- Only a small subset of us have "pull & push" access. Everyone on GitHub has "pull" access by default.

- We use pull requests for everything, even things that seem small. We have Jenkins test it before merging that into master.

- Our main repository should only have a couple branches on it: master and a few big enhancement branches.

- Small enhancements can be done on personal branches.

- All pull requests that get merged into master are tested thoroughly (test-unit, test-distributions, test-models) prior to the merge.

I think the workflow should be:

0. Fork the stan-dev/stan repository to a personal repository.

1. Update the personal/stan master repository.

2. Make changes (either to personal/stan/master or personal/stan/branch).

3. Update master, merge master into current branch (if branch==master, that's fine). Test as much needs to be tested locally.

4. Create a pull request.

For a new feature, bug fix, or anything else, start at 1.

For the pull request:

1. Someone informally code review it and declare it to be ok.

1. Start the build on Jenkins (instructions to come).

3. One of the persons with "pull & push" access merge into master and push the changes.

(Yes, that's two ones. If the pull request is from a trusted source, someone can kick off the build before reviewing.)

The workflow would be similar to the other branches we have for longer development, just replace "master" with the branch of your choice.

One thing that I find myself doing are small commits to fix tests after making a larger commit. This is for both myself and others. For those of you that think that small commits don't break the build, I've actually spent a lot of time going back and making tests pass. I've done this for literally everyone that's committed to Stan. I'm actually the last one to do this -- although the last pull request fixed one of the model tests, the other ones failed. I've had to add another quick patch to make it work. I'm running Jenkins now to verify that all the tests pass.

In early email threads I had suggested our code review be very informal, but we still have pull requests. For example, the last pull request, there isn't a need for anyone to really approve it, but it was quick and Marcus said it looked ok. I wouldn't be comfortable with even something as small as that getting pushed into master without having our tests run. If the goal is for our master to be stable, then we should enforce this behavior.

Hopefully that wasn't too long-winded. The short is, I would rather have everything go through pull requests. If the number of people that can approve it becomes the bottle neck, we can increase the number. I think the real bottle neck will be the number of pull requests that we can test on a daily basis. If Ben can help us get the testing done quicker by pushing to Hotfoot or even some other machines, I think we can deal with that burden for a while. We want to speed up our builds, but it's really f'n hard when master's tests don't pass because we weren't diligent about testing.

To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

--
You received this message because you are subscribed to the Google Groups "stan development mailing list" group.

To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+unsubscribe@googlegroups.com.

[Ben Goodrich]

On Tuesday, May 14, 2013 11:10:50 PM UTC-4, Daniel Lee wrote:

I think I've finally caught up on all the thoughts. Sorry about being unavailable the last few hours.

My opinions differ from most of what's being discussed. I'm first going to restore everyone's privledges for the time being, but I don't think this is the right thing to do. (Ben, what's your GitHub handle? Your commits aren't being picked up by GitHub as a user.)

Here is what I suggest:

- Only a small subset of us have "pull & push" access. Everyone on GitHub has "pull" access by default.

- We use pull requests for everything, even things that seem small. We have Jenkins test it before merging that into master.

- Our main repository should only have a couple branches on it: master and a few big enhancement branches.

- Small enhancements can be done on personal branches.

- All pull requests that get merged into master are tested thoroughly (test-unit, test-distributions, test-models) prior to the merge.

This sounds a lot like the sympy rules:

https://github.com/sympy/sympy/wiki/Development-workflow

and, well, they have 117 outstanding pull requests and don't release very often

http://code.google.com/p/sympy/downloads/list

It can fall into a negative feedback loop where the pull request process is a hassle, so people do fewer pull requests with more commits each, which are harder to review, and so on. If we are going to do peer review, I am pretty sure that gerrit is the consensus choice for the tool to use and although I don't have any direct experience with it, this blogpost seemed persuasive to me

http://julien.danjou.info/blog/2013/rant-about-github-pull-request-workflow-implementation

What you are saying is not incompatible with something like gitflow, but I think if we had a better workflow then the question of merging would be secondary.

Ben

[Daniel Lee]

Thanks for pulling up those links. My thoughts do line up with sympy rules pretty closely.

I guess I didn't make it clear: since I'm proposing that we do pull request and test everything, I'm volunteering to make sure things get tested and notify people when it doesn't pass tests. I wouldn't have suggested it if I wasn't willing to put in the time to make this work.

I'm not sure what the difference is between "workflow" and "merging." I think we can manage it fine with GitHub's pull requests for the time being. I don't want to introduce another tool if we don't have to. As long as I (and the other kind volunteers who want to keep master pristine) am willing to put in the time to manage pull requests, I think our current tools are sufficient.

Daniel

PS. I could be completely wrong in thinking that GitHub pull request is enough of a tool. I didn't see a use for git years ago when Ben showed it to me, but then again, at the time, the process was simple enough where SVN was sufficient.

--
You received this message because you are subscribed to the Google Groups "stan development mailing list" group.

To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.

[Ben Goodrich]

On Wednesday, May 15, 2013 1:00:24 AM UTC-4, Daniel Lee wrote:

Thanks for pulling up those links. My thoughts do line up with sympy rules pretty closely.

I guess I didn't make it clear: since I'm proposing that we do pull request and test everything, I'm volunteering to make sure things get tested and notify people when it doesn't pass tests. I wouldn't have suggested it if I wasn't willing to put in the time to make this work.

I think we need to get to a point where the person making the pull request is 99% sure that each of the commits that are in the pull request pass the tests. So, that requires the person making the pull request to do a lot of testing locally, on Hotfoot, with Jenkins, whatever. But then, how much is the reviewer of the pull request adding? Another test of the branch as a whole is redundant 99% of the time. The style stuff can be automated with hooks. The reviewer could notice when there is insufficient unit testing but is that worth it?
 

I'm not sure what the difference is between "workflow" and "merging."

Merging is small but important part of the workflow that comes at the end of the process. We need to get a lot better at the git commit level.

Ben

[Bob Carpenter]

Now that I think I understand it better, I'm down with the gitflow.
Let's see if this summary is a little easier (and on target --- Ben?).

I also think a lot of our problems will be solved by separating

--- master branch: bug fixes w.r.t. last stable release

--- dev branch: new features w.r.t. last stable release

--- feature branch: where a new feature or bug fix goes

Merge process is then:

feature branch -> master: bug fixes

feature branch -> dev: new features

dev -> master: non-patch release

We know we want master to be stable and in an always-releasable
state. But it brings up some questions:

1. How sloppy can we be on the feature branches?

2. How sloppy can we be on the dev branch?

3. How much testing do we do for each commit?

4. How much testing do we do for each merge?

I think in particular, we've been very sloppy at the commit level
compared to the recommended git practices. I find the bisect argument
and log-scanning arguments persuasive.

http://julien.danjou.info/blog/2013/rant-about-github-pull-request-workflow-implementation

Also, the above workflow, you'll see they do complete testing for integration.
The developers should do testing for commits, but merges get extra attention.

I actually think this could speed up, not slow down, our process.

An orthogonal issue is code review. I think it's a good idea to set
up code review for the same reason it's a good idea to have pair
programming. And not just for outside contribs. I'd love it if someone
could review ALL of the code I commit for both style and substance.

- Bob

[Ben Goodrich]

On Wednesday, May 15, 2013 11:17:59 AM UTC-4, Bob Carpenter wrote:

Now that I think I understand it better, I'm down with the gitflow.
Let's see if this summary is a little easier (and on target --- Ben?).

I also think a lot of our problems will be solved by separating

    --- master branch:  bug fixes w.r.t. last stable release

    --- dev branch:  new features w.r.t. last stable release

    --- feature branch:  where a new feature or bug fix goes

Almost, there exists the concept of a hotfix branch that is not a feature branch. A hotfix branch would often just have one commit and is merged into both the development branch and the master branch (at which point you could do a release if the bug were severe enough). For what it is worth, a feature branch would regularly git pull from the development branch as the feature is being developed and thus would get the hotfixes as they arise.
 

Merge process is then:

    feature branch -> master: bug fixes

 

    feature branch -> dev:  new features

hotfix branch -> master and development
feature branch -> development only
 

    dev -> master: non-patch release

this would probably entail a nonminimal bump of the version number
 

We know we want master to be stable and in an always-releasable
state.  But it brings up some questions:

1.  How sloppy can we be on the feature branches?

2.  How sloppy can we be on the dev branch?

3.  How much testing do we do for each commit?

4.  How much testing do we do for each merge?

I think in particular, we've been very sloppy at the commit level
compared to the recommended git practices.  I find the bisect argument
and log-scanning arguments persuasive.

We certainly can't be sloppy at all on a hotfix branch, nor a merge from development into master. The development branch doesn't need to be in a releasable state always, but its history will become part of master's history eventually. So, I still think you want to make sure each commit is sound, although maybe not obsessing about whether it works on all platforms, toolchains, optimization levels, etc. With a feature branch, you can be "sloppy" initially because no one should be branching from a feature branch (feature branches branch from the development branch). So, you can implement a feature in a feature branch and rewrite your history before merging it into the development branch. But it is easier to get things right the first time ...

Ben

[Jiqiang Guo]

On Wed, May 15, 2013 at 11:44 AM, Ben Goodrich <goodri...@gmail.com> wrote:

So, you can implement a feature in a feature branch and rewrite your history before merging it into the development branch

How to rewrite history? git rebase?

--

Jiqiang

[Ben Goodrich]

Mostly, yes. Or if you just need to rewrite the previous commit, then

git commit --amend

Or, if

git rebase -i

is not what you want, you can do

git diff development > /tmp/big_patch.diff
git checkout development
git branch -D feature
git branch feature
git checkout feature
git apply /tmp/big_patch.diff

and then add and commit appropriate chunks of your big_patch as the new history.

The main thing is to not rewrite history if someone else has already branched from some point in the old history because then they can't merge. So, you can only change the history of local branches and possibly public feature branches (because feature branches should generally branch directly from the development branch).

Ben

[Marcus Brubaker]

On Wed, May 15, 2013 at 11:44 AM, Ben Goodrich <goodri...@gmail.com> wrote:
> On Wednesday, May 15, 2013 11:17:59 AM UTC-4, Bob Carpenter wrote:
>>
>>
>> dev -> master: non-patch release
>
>
> this would probably entail a nonminimal bump of the version number

So technically the gitflow process says only release branches get
merged to master. In other words:

1. Merge/commit new features/bug fixes in the dev branch, until dev
settles down and we're ready to make a release.

2. Branch from dev to create a release N+1 branch, update docs/version
numbers/etc, run full battery of tests and create release.

3. Merge release branch back into master.

A hotfix would instead branch directly off of master instead of dev.

>
>>
>>
>> We know we want master to be stable and in an always-releasable
>> state. But it brings up some questions:
>>
>> 1. How sloppy can we be on the feature branches?
>>
>> 2. How sloppy can we be on the dev branch?
>>
>> 3. How much testing do we do for each commit?
>>
>> 4. How much testing do we do for each merge?
>>
>> I think in particular, we've been very sloppy at the commit level
>> compared to the recommended git practices. I find the bisect argument
>> and log-scanning arguments persuasive.
>
>
> We certainly can't be sloppy at all on a hotfix branch, nor a merge from
> development into master. The development branch doesn't need to be in a
> releasable state always, but its history will become part of master's
> history eventually. So, I still think you want to make sure each commit is
> sound, although maybe not obsessing about whether it works on all platforms,
> toolchains, optimization levels, etc. With a feature branch, you can be
> "sloppy" initially because no one should be branching from a feature branch
> (feature branches branch from the development branch). So, you can implement
> a feature in a feature branch and rewrite your history before merging it
> into the development branch. But it is easier to get things right the first
> time ...

Just so everyone is clear, we're really talking about two different
things here. They're related and both of them should be addressed,
but they are distinct.

1. The gitflow discussion relates to how we manage releases and the
main Stan repository. What goes where, when and in what state. What
do different branches mean, when do they merge, and in what direction,
etc. In other words, how all of us work jointly on Stan.

2. The commit history discussion relates to how we work locally.
Ideally every commit should build and, hopefully, pass unit tests. I
think that's a good goal to keep in mind but a little overly
optimistic, and perhaps even a little counter intuitive because it
will lead to Balkanization of commits. I.E., instead of committing
often with small changes, this encourages large block commits which
have been fully vetted and makes git bisect similarly useless. (If it
comes down to a single commit, but that commit touches dozens of files
and changes hundreds of lines of code, that's not terribly helpful.)

For 1) I like the gitflow process, it seems to makes sense, and
ensures that we can always do a release with a bug fix if needed.

For 2) I see Ben has already posted something, but roughly speaking,
it means before we do a git push, we should do a git rebase -i and
clean up our commits. It's also probably a good idea to do the git
rebase regularly for longer lasting feature branches to keep your
history clean since doing a rebase at the end of a long development
cycle can be painful and require a lot of manual merging.

Cheers,
Marcus

[Bob Carpenter]

On 5/15/13 12:41 PM, Marcus Brubaker wrote:
> On Wed, May 15, 2013 at 11:44 AM, Ben Goodrich <goodri...@gmail.com> wrote:
>> On Wednesday, May 15, 2013 11:17:59 AM UTC-4, Bob Carpenter wrote:
>>>
>>>
>>> dev -> master: non-patch release
>>
>>
>> this would probably entail a nonminimal bump of the version number
>
> So technically the gitflow process says only release branches get
> merged to master. In other words:
>
> 1. Merge/commit new features/bug fixes in the dev branch, until dev
> settles down and we're ready to make a release.
>
> 2. Branch from dev to create a release N+1 branch, update docs/version
> numbers/etc, run full battery of tests and create release.
>
> 3. Merge release branch back into master.
>
> A hotfix would instead branch directly off of master instead of dev.

That's my understanding of the process, and I finally understand
why it makes sense now that we have so many different branches
of development.

>>> We know we want master to be stable and in an always-releasable
>>> state. But it brings up some questions:
>>>
>>> 1. How sloppy can we be on the feature branches?

We want to keep the commits organized and/or rebase them.
I'm still a little hazy on the arguments pro and con here.

If we keep the branches and *don't allow fast-forward*, won't
that still support bisect in the cases where you don't have
good commits on the branch?

>>> 2. How sloppy can we be on the dev branch?

Not at all. We want things to go onto the dev branch when they
work. If they don't work, they get kicked off the dev branch
until they do.

>>> 3. How much testing do we do for each commit?

Same question as 1. The commits are on the feature branches.

>>> 4. How much testing do we do for each merge?

Same question as 2. Enough to keep dev in working order.

Assuming we test everything on whatever our local setup is
before merging with dev, then the only issues likely to come
up are (a) race conditions with other merges, and (b) incompatibility
with other platforms/optimization levels.

I'm still not sure how to deal with that.

>>> I think in particular, we've been very sloppy at the commit level
>>> compared to the recommended git practices. I find the bisect argument
>>> and log-scanning arguments persuasive.
>>
>>
>> We certainly can't be sloppy at all on a hotfix branch, nor a merge from
>> development into master. The development branch doesn't need to be in a
>> releasable state always, but its history will become part of master's
>> history eventually. So, I still think you want to make sure each commit is
>> sound, although maybe not obsessing about whether it works on all platforms,
>> toolchains, optimization levels, etc. With a feature branch, you can be
>> "sloppy" initially because no one should be branching from a feature branch
>> (feature branches branch from the development branch). So, you can implement
>> a feature in a feature branch and rewrite your history before merging it
>> into the development branch. But it is easier to get things right the first
>> time ...

So does that mean the answer is "no", we won't be able to do bisect
unless we rebase our merges so that they look like single events?
Doesn't the branching w/o fastforward do that?

> Just so everyone is clear, we're really talking about two different
> things here. They're related and both of them should be addressed,
> but they are distinct.
>
> 1. The gitflow discussion relates to how we manage releases and the
> main Stan repository. What goes where, when and in what state. What
> do different branches mean, when do they merge, and in what direction,
> etc. In other words, how all of us work jointly on Stan.

Agreed. I'd also add

3 permissions. Who has permission to do what and what the code
review policies are before merges happen.

That is, do we make everything be a pull request that an admin
manages before the merge with dev happens?

I think we can all agree that we should be strict about what goes
into master and when.

4. code review. What is our process for reviewing code if any.
Does a third party have to sign off on a code review for every
pull request before an attempt is made to merge with dev and test?

The gitflow is agnostic on that point, and the minimal thing to do is
just let everything merge back into dev that passes all tests.

What I'm worried about specifically on this issue is new
features getting added without tests, as that seems to be happening
more and more often (I'm not trying to point fingers here --- I'm as
guilty as anyone of this; I was used to checkpointing regularly in
SVN this way, but it makes less sense in git to do this outside of
your own branch). For instance, some of the pending pull requests
are still pending because there aren't enough tests of the added
features.

Now what I'd like to be able to do is go into that pull request
and modify it to add the tests myself, wrap it up as another

> 2. The commit history discussion relates to how we work locally.
> Ideally every commit should build and, hopefully, pass unit tests. I
> think that's a good goal to keep in mind but a little overly
> optimistic, and perhaps even a little counter intuitive because it
> will lead to Balkanization of commits. I.E., instead of committing
> often with small changes, this encourages large block commits which
> have been fully vetted and makes git bisect similarly useless. (If it
> comes down to a single commit, but that commit touches dozens of files
> and changes hundreds of lines of code, that's not terribly helpful.)

We can be sloppy on this and fix it all up later by rebasing.
But what I'm wondering is whether that's necessary if we keep the
branch history as a branch instead of fast fowarding (that is, keep
it as a branch rather than merging its local commits into dev and
then into master).

> For 1) I like the gitflow process, it seems to makes sense, and
> ensures that we can always do a release with a bug fix if needed.

Agreed. I think I finally understand it.

> For 2) I see Ben has already posted something, but roughly speaking,
> it means before we do a git push, we should do a git rebase -i and
> clean up our commits. It's also probably a good idea to do the git
> rebase regularly for longer lasting feature branches to keep your
> history clean since doing a rebase at the end of a long development
> cycle can be painful and require a lot of manual merging.

Why does a rebase require any manual merging at all? Don't we just
compose the patches into one big patch?

Finally, there's the issue of where we set all of this up. Daniel
and I have been talking about isolating the dev and master branches
in something like stanfoo where only a couple people have permission
to control the merge, and then creating a new repository stanbar where
we actually work on all the branches. I'm still not sure the latter
will work if we have to clone a whole remote repository (i.e., all of
stanfoo) rather than just being able to branch from a remote branch
(i.e., branching from stanfoo/dev for features or stanfoo/master for
hotfixes).

- Bob

[Marcus Brubaker]

This is *exactly* what I worry about with doing everything through
pull requests. There are many many commits which are small and
obvious and do not need a full process of review/compile/test. If we
make even these commits hard or time-consuming to do, they won't
happen at all or will instead get folded into much larger pull
requests whose correctness aren't at all obvious.

If we're going to require *every* commit to compile and pass unit
tests then we need some a bigger testing infrastructure (i.e., servers
which test every commit in every pull request, automatically and,
ideally, quickly) to do this and it needs to be smooth and painless.
I'm not convinced this is possible with GitHub's pull request
framework, as it seems to struggle with anything other than an
immediate merge. E.G., if one commit in a large sequence breaks
tests, this is going to require a rebase and a whole new pull request
which is not only tedious, but looses the whole thread of discussion
in the issue tracker.

I really think the solution here is that we all just try to be better
about our workflows and try to make things clean when we do push.
That, combined with a disciplined version of something like gitflow,
should make things much smoother going forward.

Cheers,
Marcus

[Bob Carpenter]

On 5/15/13 2:40 PM, Marcus Brubaker wrote:
> On Wed, May 15, 2013 at 12:51 AM, Ben Goodrich <goodri...@gmail.com> wrote:
>> On Tuesday, May 14, 2013 11:10:50 PM UTC-4, Daniel Lee wrote:

>> It can fall into a negative feedback loop where the pull request process is
>> a hassle, so people do fewer pull requests with more commits each, which are
>> harder to review, and so on.
>
> This is *exactly* what I worry about with doing everything through
> pull requests.

This does sound like it'll be problematic. This is where
Daniel and I aren't quite on the same page yet.

> There are many many commits which are small and
> obvious and do not need a full process of review/compile/test.

But this we both agree on. Even small commits which seem obvious
should be reviewed, compiled and tested. Programming's just too
hard and it's just too easy to get fooled into thinking something's
simple that isn't. At least I get fooled easily and would actually
prefer to have all this stuff reviewed by a second pair of eyes.

I think Daniel and I are volunteering to do most of these reviews.

It does not seem like the GitHub pull request mechanism is the easiest
option out there. (Well, it's easiest in that it's already there
and doesn't need a tool like Gerrit, but not easiest for the reasons
you cite below.)

So I think it's this balance that we're trying to achieve.

The other issue is that it's impossible for each user to do all the
tests locally if they don't have a way to fire off some cluster of
machines, operating systems and compilers to do a full battery of
tests. We don't do that now, so there's still a big danger in our
current process of releasing something that only works on Windows
and Mac at a couple of optimization levels, because that's all Daniel
and I test before the releases.

> If we
> make even these commits hard or time-consuming to do, they won't
> happen at all or will instead get folded into much larger pull
> requests whose correctness aren't at all obvious.
>
> If we're going to require *every* commit to compile and pass unit
> tests then we need some a bigger testing infrastructure (i.e., servers
> which test every commit in every pull request, automatically and,
> ideally, quickly) to do this and it needs to be smooth and painless.

Agreed. Both to the person submitting the fix and to the people
reviewing it.

> I'm not convinced this is possible with GitHub's pull request
> framework, as it seems to struggle with anything other than an
> immediate merge. E.G., if one commit in a large sequence breaks
> tests, this is going to require a rebase and a whole new pull request
> which is not only tedious, but looses the whole thread of discussion
> in the issue tracker.
>
> I really think the solution here is that we all just try to be better
> about our workflows and try to make things clean when we do push.
> That, combined with a disciplined version of something like gitflow,
> should make things much smoother going forward.

At the very least, even tiny little patches need to be thoroughly tested
locally before any attempt is made to merge them. We've already shown
that we're not very disciplined as far as this goes and we'll be getting
more and more users, not all of whom have hours to discuss process and
understand our honor system of standard operating procedure.

So what we're trying to do is find a workable balance that:

1. leaves master in a releasable state,

2. raises an alarm whenever dev doesn't pass all tests on all platforms,
so that changes can be backed out (or never added) until they work, and

3. lets people work on features with a minimum of pain.

- Bob

[Ben Goodrich]

On Wednesday, May 15, 2013 2:52:03 PM UTC-4, Bob Carpenter wrote:

On 5/15/13 2:40 PM, Marcus Brubaker wrote:
> On Wed, May 15, 2013 at 12:51 AM, Ben Goodrich <goodri...@gmail.com> wrote:
>> On Tuesday, May 14, 2013 11:10:50 PM UTC-4, Daniel Lee wrote:

>> It can fall into a negative feedback loop where the pull request process is
>> a hassle, so people do fewer pull requests with more commits each, which are
>> harder to review, and so on.
>
> This is *exactly* what I worry about with doing everything through
> pull requests.

This does sound like it'll be problematic.  This is where
Daniel and I aren't quite on the same page yet.

I am pretty much in agreement with Marcus. You are definitely going to need human input on

1) Merges of hotfixes into master
2) Merges of release branches into master
3) Merges of feature branches into development

and I think it is possible so that only you or Daniel can push the big red button on merges like that. But what we really need is consensus that these merges are going to be okay.

In addition, I think little if any external scrutiny needs to be enforced on commits that only touch src/test, src/model, and / or src/doc, provided that the author has done some reasonable testing. If such commits are flawed, it is not that big a deal anyway. Much more scrutiny is needed for commits that touch src/stan or make/.
 

The other issue is that it's impossible for each user to do all the
tests locally if they don't have a way to fire off some cluster of
machines, operating systems and compilers to do a full battery of
tests.

Haven't heard back from Hotfoot about this yet. Amazon EC2 is another option if we have money for it.

Ben

[Marcus Brubaker]

On Wed, May 15, 2013 at 2:38 PM, Bob Carpenter <ca...@alias-i.com> wrote:
>>>> We know we want master to be stable and in an always-releasable
>>>> state. But it brings up some questions:
>>>>
>>>> 1. How sloppy can we be on the feature branches?
>
>
> We want to keep the commits organized and/or rebase them.
> I'm still a little hazy on the arguments pro and con here.
>
> If we keep the branches and *don't allow fast-forward*, won't
> that still support bisect in the cases where you don't have
> good commits on the branch?

The --no-ff option simply forces git to create a new commit which says
"Merging foo into bar." even if it's not required. It doesn't compact
the merge into a single commit, the full history is kept.

>
>
>>>> 3. How much testing do we do for each commit?
>
>
> Same question as 1. The commits are on the feature branches.

But when the feature branches are merged onto dev, that commit history
is kept. This is definitely a good thing, see below.

>>>> 4. How much testing do we do for each merge?
>
>
> Same question as 2. Enough to keep dev in working order.
>
> Assuming we test everything on whatever our local setup is
> before merging with dev, then the only issues likely to come
> up are (a) race conditions with other merges, and (b) incompatibility
> with other platforms/optimization levels.
>
> I'm still not sure how to deal with that.

I think (a) and (b) are *exactly* what dev is for, sorting these
issues out. If every feature was perfect when it was ready to merge,
then why even bother with dev? Why not merge straight to the next
release branch?

>
>
>>>> I think in particular, we've been very sloppy at the commit level
>>>> compared to the recommended git practices. I find the bisect argument
>>>> and log-scanning arguments persuasive.
>>>
>>>
>>>
>>> We certainly can't be sloppy at all on a hotfix branch, nor a merge from
>>> development into master. The development branch doesn't need to be in a
>>> releasable state always, but its history will become part of master's
>>> history eventually. So, I still think you want to make sure each commit
>>> is
>>> sound, although maybe not obsessing about whether it works on all
>>> platforms,
>>> toolchains, optimization levels, etc. With a feature branch, you can be
>>> "sloppy" initially because no one should be branching from a feature
>>> branch
>>> (feature branches branch from the development branch). So, you can
>>> implement
>>> a feature in a feature branch and rewrite your history before merging it
>>> into the development branch. But it is easier to get things right the
>>> first
>>> time ...
>
>
> So does that mean the answer is "no", we won't be able to do bisect
> unless we rebase our merges so that they look like single events?
> Doesn't the branching w/o fastforward do that?

A git bisect which finds that the error occured when the
giant-feature-branch-which-touches-everything was merged is exactly as
useless as one for which the results is ambiguous due to a compilation
failure for a huge sequence of commits. We want to keep this history,
even if it is somewhat imperfect.

>
>
>> Just so everyone is clear, we're really talking about two different
>> things here. They're related and both of them should be addressed,
>> but they are distinct.
>>
>> 1. The gitflow discussion relates to how we manage releases and the
>> main Stan repository. What goes where, when and in what state. What
>> do different branches mean, when do they merge, and in what direction,
>> etc. In other words, how all of us work jointly on Stan.
>
>
> Agreed. I'd also add
>
> 3 permissions. Who has permission to do what and what the code
> review policies are before merges happen.
>
> That is, do we make everything be a pull request that an admin
> manages before the merge with dev happens?
>
> I think we can all agree that we should be strict about what goes
> into master and when.

Definitely. I think we could also agree about what goes into (the
currently hypothetical) dev and when.

>
> 4. code review. What is our process for reviewing code if any.
> Does a third party have to sign off on a code review for every
> pull request before an attempt is made to merge with dev and test?
>
> The gitflow is agnostic on that point, and the minimal thing to do is
> just let everything merge back into dev that passes all tests.
>
> What I'm worried about specifically on this issue is new
> features getting added without tests, as that seems to be happening
> more and more often (I'm not trying to point fingers here --- I'm as
> guilty as anyone of this; I was used to checkpointing regularly in
> SVN this way, but it makes less sense in git to do this outside of
> your own branch). For instance, some of the pending pull requests
> are still pending because there aren't enough tests of the added
> features.
>
> Now what I'd like to be able to do is go into that pull request
> and modify it to add the tests myself, wrap it up as another

This can be done now. You can pull that branch into your own local
repository, add some tests and then merge. In fact, this is something
we need to be comfortable doing as we get more contributors who may be
happy to add some new feature, but may lack the
skills/knowledge/motivation/etc to fully doc or test it. If we can't
do this to some extent, then we need to be OK with loosing
contributions like those.

>> 2. The commit history discussion relates to how we work locally.
>> Ideally every commit should build and, hopefully, pass unit tests. I
>> think that's a good goal to keep in mind but a little overly
>> optimistic, and perhaps even a little counter intuitive because it
>> will lead to Balkanization of commits. I.E., instead of committing
>> often with small changes, this encourages large block commits which
>> have been fully vetted and makes git bisect similarly useless. (If it
>> comes down to a single commit, but that commit touches dozens of files
>> and changes hundreds of lines of code, that's not terribly helpful.)
>
>
> We can be sloppy on this and fix it all up later by rebasing.
> But what I'm wondering is whether that's necessary if we keep the
> branch history as a branch instead of fast fowarding (that is, keep
> it as a branch rather than merging its local commits into dev and
> then into master).

As a rule (without extreme pain) we can't rebase anything that has
landed into dev, master or any other branch that we expect people to
branch off of. Rebase rewrites history in a way that breaks anyone
who branched from that history.

>> For 2) I see Ben has already posted something, but roughly speaking,
>> it means before we do a git push, we should do a git rebase -i and
>> clean up our commits. It's also probably a good idea to do the git
>> rebase regularly for longer lasting feature branches to keep your
>> history clean since doing a rebase at the end of a long development
>> cycle can be painful and require a lot of manual merging.
>
>
> Why does a rebase require any manual merging at all? Don't we just
> compose the patches into one big patch?

A rebase attempts to replay some or all of your commits as if they
were applied starting from a different point. Sometimes that doesn't
happen cleanly because of conflicts. Think of a rebase like this:

Imagine you have a sequence of 10 commits. Commit 1 was originally
off of the HEAD of master, but there was another commit on master, so
it has a new HEAD and you want commits 1 through 10 to look as if they
happened on this new HEAD and you'd like to combine or reorder some of
them to make things look nicer.

To do this, git rebase creates a sequence of patches from your 10
commits, goes to the new head that you're trying to rebase onto and
then tries to replay these patches on this new head, one at a time, to
create a new set of commits. There can often be conflicts in this
process.

>
> Finally, there's the issue of where we set all of this up. Daniel
> and I have been talking about isolating the dev and master branches
> in something like stanfoo where only a couple people have permission
> to control the merge, and then creating a new repository stanbar where
> we actually work on all the branches. I'm still not sure the latter
> will work if we have to clone a whole remote repository (i.e., all of
> stanfoo) rather than just being able to branch from a remote branch
> (i.e., branching from stanfoo/dev for features or stanfoo/master for
> hotfixes).

If there was a way that we could add repos/branches for Jenkins to
watch and send us updates on, that would be great. Particularly if it
didn't require a pull request, as I'd prefer not to have to open a
browser and login to github every time.

Cheers,
Marcus

[Ben Goodrich]

On Wednesday, May 15, 2013 3:11:36 PM UTC-4, Ben Goodrich wrote:

and I think it is possible so that only you or Daniel can push the big red button on merges

Bob and Daniel have been talking about this offlist. Here is a link

https://www.kernel.org/pub/software/scm/git/docs/howto/update-hook-example.txt

Ben

[Daniel Lee]

We don't have access to hooks, as far as I can tell. They have a bunch of service hooks, but not the git-core hooks.

--
You received this message because you are subscribed to the Google Groups "stan development mailing list" group.

To unsubscribe from this group and stop receiving emails from it, send an email to stan-dev+u...@googlegroups.com.

[Ben Goodrich]

On Wednesday, May 15, 2013 3:20:43 PM UTC-4, Daniel Lee wrote:

We don't have access to hooks, as far as I can tell. They have a bunch of service hooks, but not the git-core hooks.

Can't we just put one into STAN_HOME/.git/hooks ?

Ben

[Daniel Lee]

Can you verify that it works with a personal repository?

It looks like post-receive hooks are enabled through a web interface: https://help.github.com/articles/post-receive-hooks

But they don't really support the git-core hooks.

[Bob Carpenter]

On 5/15/13 3:12 PM, Marcus Brubaker wrote:
> On Wed, May 15, 2013 at 2:38 PM, Bob Carpenter <ca...@alias-i.com> wrote:
>>>>> We know we want master to be stable and in an always-releasable
>>>>> state. But it brings up some questions:
>>>>>
>>>>> 1. How sloppy can we be on the feature branches?
>>
>>
>> We want to keep the commits organized and/or rebase them.
>> I'm still a little hazy on the arguments pro and con here.
>>
>> If we keep the branches and *don't allow fast-forward*, won't
>> that still support bisect in the cases where you don't have
>> good commits on the branch?
>
> The --no-ff option simply forces git to create a new commit which says
> "Merging foo into bar." even if it's not required. It doesn't compact
> the merge into a single commit, the full history is kept.

My question is what happens to bisect when run on dev.
I thought we wound up with something like this:

DEV: a ---------------> b
\ ^
\ /
BRANCH c ---> d --> e


but with fast forward, we get:

a -> c -> d -> e

When we bisect on DEV in the first case, does it only consider
a and b, or does it also look at c, d, and e?

>>>>> 3. How much testing do we do for each commit?
>>
>>
>> Same question as 1. The commits are on the feature branches.
>
> But when the feature branches are merged onto dev, that commit history
> is kept. This is definitely a good thing, see below.
>
>>>>> 4. How much testing do we do for each merge?
>>
>>
>> Same question as 2. Enough to keep dev in working order.
>>
>> Assuming we test everything on whatever our local setup is
>> before merging with dev, then the only issues likely to come
>> up are (a) race conditions with other merges, and (b) incompatibility
>> with other platforms/optimization levels.
>>
>> I'm still not sure how to deal with that.
>
> I think (a) and (b) are *exactly* what dev is for, sorting these
> issues out. If every feature was perfect when it was ready to merge,
> then why even bother with dev? Why not merge straight to the next
> release branch?

Because there may be a whole bunch of features going out in the
next incremental release (say 1.1.0 to 1.2.0), but they may not be
done for a month or two. Then you have to hotfix master due to
some critical bug you want to get out to users in a 1.1.1 release.

I was sort of hoping that the bisect on dev would find which branch
introduced the problem, then bisecting that branch would find the commit.

The latter is only going to work if all the little commits on that
branch are good. But the former's useful to at least narrow down where
the diverging behavior was introduced.

It's a delicate balance here. Testing and doc can take longer than
writing the feature. And then the problem is if the test fails, who
fixes it?

That's where we're blocked on one of the pull requests -- not enough tests.
I can get around eventually to adding more tests for it, but it's
not a high priority because the feature itself wasn't a high priority.

So we'll get a pileup of incomplete pull requests that we can prioritize
and work on ourselves just reject if the requester isn't willing to do
the work. I'm leaning more toward the latter these days as I'm feeling
totally overwhelmed with (a) this build process, and (b) the new features
I'm down for, like discrete sampling.

>>> 2. The commit history discussion relates to how we work locally.
>>> Ideally every commit should build and, hopefully, pass unit tests. I
>>> think that's a good goal to keep in mind but a little overly
>>> optimistic, and perhaps even a little counter intuitive because it
>>> will lead to Balkanization of commits. I.E., instead of committing
>>> often with small changes, this encourages large block commits which
>>> have been fully vetted and makes git bisect similarly useless. (If it
>>> comes down to a single commit, but that commit touches dozens of files
>>> and changes hundreds of lines of code, that's not terribly helpful.)
>>
>>
>> We can be sloppy on this and fix it all up later by rebasing.
>> But what I'm wondering is whether that's necessary if we keep the
>> branch history as a branch instead of fast fowarding (that is, keep
>> it as a branch rather than merging its local commits into dev and
>> then into master).
>
> As a rule (without extreme pain) we can't rebase anything that has
> landed into dev, master or any other branch that we expect people to
> branch off of. Rebase rewrites history in a way that breaks anyone
> who branched from that history.

I thought the idea was to rebase the branch before merging it.

I've only thought about this through bisect --- are there other implications?
I'm not sure what "rewrites history" means here. (Or what Ben meant about
backing out features creating a circular history.)

>>> For 2) I see Ben has already posted something, but roughly speaking,
>>> it means before we do a git push, we should do a git rebase -i and
>>> clean up our commits. It's also probably a good idea to do the git
>>> rebase regularly for longer lasting feature branches to keep your
>>> history clean since doing a rebase at the end of a long development
>>> cycle can be painful and require a lot of manual merging.
>>
>>
>> Why does a rebase require any manual merging at all? Don't we just
>> compose the patches into one big patch?
>
> A rebase attempts to replay some or all of your commits as if they
> were applied starting from a different point. Sometimes that doesn't
> happen cleanly because of conflicts. Think of a rebase like this:
>
> Imagine you have a sequence of 10 commits. Commit 1 was originally
> off of the HEAD of master, but there was another commit on master, so
> it has a new HEAD and you want commits 1 through 10 to look as if they
> happened on this new HEAD and you'd like to combine or reorder some of
> them to make things look nicer.
>
> To do this, git rebase creates a sequence of patches from your 10
> commits, goes to the new head that you're trying to rebase onto and
> then tries to replay these patches on this new head, one at a time, to
> create a new set of commits. There can often be conflicts in this
> process.

Thanks for the example. I didn't realize this is what it was doing.

>> Finally, there's the issue of where we set all of this up. Daniel
>> and I have been talking about isolating the dev and master branches
>> in something like stanfoo where only a couple people have permission
>> to control the merge, and then creating a new repository stanbar where
>> we actually work on all the branches. I'm still not sure the latter
>> will work if we have to clone a whole remote repository (i.e., all of
>> stanfoo) rather than just being able to branch from a remote branch
>> (i.e., branching from stanfoo/dev for features or stanfoo/master for
>> hotfixes).
>
> If there was a way that we could add repos/branches for Jenkins to
> watch and send us updates on, that would be great. Particularly if it
> didn't require a pull request, as I'd prefer not to have to open a
> browser and login to github every time.

I don't understand what you're asking here. Updates about what?

Gack. It does look like GitHub only lets you do this through the
browser. I'm with you and hate working through browsers compared
to the command line. This is all very theoretical for me as I've
never successfully created a pull request myself. It looks like there
are GitHub projects implementing command-line tools for pull requests,
but I have no idea how stable they are.

- Bob

[Ben Goodrich]

On Wednesday, May 15, 2013 3:30:45 PM UTC-4, Daniel Lee wrote:

Can you verify that it works with a personal repository?

It looks like post-receive hooks are enabled through a web interface: https://help.github.com/articles/post-receive-hooks

But they don't really support the git-core hooks.

I guess a post-receive hook is not going to help us much. But if a limited number of people have access to the GitHub repo and all of those people except you and Bob have a pre-push hook (requires git >= 1.8.0 I think) or similar in their STAN_HOME/.git/hooks that denies a push into origin/{master,development}, I think it would be okay. Maybe your and Bob's pre-push hook could require you to confirm that you want to do such a merge.

Ben

[Ben Goodrich]

On Wednesday, May 15, 2013 3:37:33 PM UTC-4, Bob Carpenter wrote:

My question is what happens to bisect when run on dev.
I thought we wound up with something like this:

   DEV:  a ---------------> b
           \                ^
            \              /
BRANCH      c ---> d --> e


but with fast forward, we get:

          a -> c -> d -> e

When we bisect on DEV in the first case, does it only consider
a and b, or does it also look at c, d, and e?

git bisect is a commit level concept rather than a branch concept, so it should drill down to the first bad commit in the history between the bracket commits. Although, git bisect would require fewer tests to find the first bad commit if there were a nice simple history between the brackets, it will find it eventually as long as all the commits can definitively test for the problem (as opposed to bombing because of a missing semicolon in a commit)

I was sort of hoping that the bisect on dev would find which branch
introduced the problem, then bisecting that branch would find the commit.

No, it just finds the commit.
 

> As a rule (without extreme pain) we can't rebase anything that has

> landed into dev, master or any other branch that we expect people to
> branch off of.  Rebase rewrites history in a way that breaks anyone
> who branched from that history.

I thought the idea was to rebase the branch before merging it.

No one should rebase something that someone else could reasonably have branched from (due to the impossibility of merging later). So, you can't rebase the development branch before branching a release branch from it since people will have branched from development to make a feature branch. That person can usually rebase a feature branch before it is merged into the development branch.
 

I've only thought about this through bisect --- are there other implications?
I'm not sure what "rewrites history" means here.  (Or what Ben meant about
backing out features creating a circular history.)

The main implication is the ability to merge rather than the ability to bisect. There are a couple of other ways to rewrite history that I mentioned to Mav, namely git commit --amend and making a patch, applying that patch to a new branch, and adding / committing chunks at a time.

Ben