@seantalts, I’m all for it, but it doesn’t solve the actual problem. I’ll describe the problem and hopefully we can find a solution around it.
The reason for having contributors add to a branch was to make permissions easier. Almost all contributions to the math and stan libraries from external developers needed (and still need) a lot of core developer help to get it to be incorporated into the library. The help usually came from myself or @Bob_Carpenter , but occasionally help would come from @bgoodri, @rtrangucci, @randommm, or someone else on the core team. When the branch is on
stan-dev, we all have permissions to make changes there directly. When the branch is on a fork, individuals must be given permissions to work on the fork (unless permissioning has changed with GitHub recently).
Now, with reviews for GitHub pull requests, this might be less of an issue. We used to have to indicate what file and line number to make changes. Now we can just mark up the pull request. But, it has always been much easier to go onto branches and fix minor things as it comes up. Things like typos, cpplint, and doxygen. In the past, I’ve taken the proactive approach of helping new and existing developers get their pull requests up to speed so we can get things merged in. I understand that things like style can be very confusing and feels like an arbitrary set of rules until you read enough. For new contributors, I didn’t want this to be the issue for not being able to contribute. Without helping out, I think we would have lost a few pull requests.
So, the problem is the balance between:
- permissions for having existing core developers help with branches.
- Having a pull request come from a branch on
stan-dev means any one of us can help it along, but requires a little more administration.
- Having a pull request come from a fork means that it’s easier to submit, but if any one of us needs help out, there’s an administrative burden placed on both the team member and the submitter. Either the submitter can give permissions to the individual team member or the team member needs to submit a pull request, which requires branching from the fork, then going through the process of submitting a pull request from that branch to the fork, then the submitter needs to review and merge the pull request.
- how much the team helps with the pull request
- By putting in on a branch under a
stan-dev repo, we can help it along. From the past, this has been absolutely necessary to get new contributors, at least to Math and Stan. Right now, we’re getting contributors that are statisticians and scientists adding features that help them with their projects, so typically C++ isn’t their first language. We are not getting C++ experts (with the exception of maybe @vincent-picaud and @seantalts) to contribute solid code that adheres to standards.
- By having a pull request on a fork, the easiest thing we can do as a team is to review the code and point out problems. It’s not natural to provide help and support through pull request reviews, but it’s not impossible–it just takes a lot more effort on the part of the core developer that’s reviewing. I find it easier to help new developers along and not let cpplint be the limiting factor to submitting a pull request, but we could get the opinion of newer developers to know whether the direct help is even useful.
Regarding Jenkins: we always had a workaround. It’s essentially adding the
sha from the fork to a branch on
stan-dev and that’s enough to get all the upstream tests working.
All that said, I’d rather make things easier for the contributor. We also don’t get that many new contributors, but our heavy-ish process can’t be helping. Of course, being written in heavily templated C++ can’t help either.