Contributor attribution

[mallezie]

Hello there,

Since one year i'm using CiviCRM, when working with the software i noticed some bugs, and filed issues for them. For some bugs i created PR's to help fix them. Lately my first code changes got merged, and i even managed to get on the contributors list on the githb project. (Hurray for me ;-) ).
This is the heart of every open source project, and it's really great to have a community which is very welcoming issues, and PR's. (Hurray for CiviCRM).
However coming from the Drupal Community, and their issue trackers and patch workflow I noticed that the contribution list on the Github project is'nt really as accurate as it should be.
In the drupal community there is (always) discussion on the issue and patch workflow, and working on drupal.org, versus moving to github with it's PR-workflow. The main reason is that it's much harder to give proper attribution when working with multiple persons on the same issue.
This is also sometimes the case in the civicrm repository, where you sometimes create a PR request with a fix for the issue, and sometimes that PR causes a new PR with someone adding some more fixes to it. (This is great off course, collaboration FTW). However the first person loses attribution when this second (better) PR got merged.
If i'm not mistaking the github correct approach would be to create a new PR against the first persons PR, which the first person merges, and the first PR get's merged inside the core project. Even better is to create a first public PR, so everyone can commit in that PR. I hope my reasoning is correct here.

Could we somehow enforce this approach more in CiviCRM?
This should give better attribution, and motivates people to help out more, since you get correctly credited (and can show you civicrm expertise through your github profile). It's sometimes (a little, first world problem) frustrating to see your work merged, and not getting attribution, which can help showing your civicrm knowledge more.

Greetings Tim, and now back to fixing issues, and filing PRs ;-)

[Coleman Watts]

Firstly, welcome to the CiviCRM community, we're glad to have you here! And thank you for contributing back.

I think some people at Civi (having the migration from SVN still in recent memory) are still adjusting to GitHub and learning best practices.

I'm curious about your idea of a "public" PR - I don't know of a way to do that, I believe github requires you to explicitly grant permissions to people one-at-a-time. If that's not the case I'd be very interested to learn.

Barring that, in the example you described I believe the "best" way to merge your PR with additional changes, maintaining attribution, would be for someone (me for example) to:

• Add your remote and checkout your branch locally
• Add a new commit
• Create a new pull-request. It will contain the commits from your original PR (attributed to you) plus my extra commit(s) (attributed to me).
• Close your original pull-request and merge the new one.

The "hub" tool makes this quite easy, but not everyone may be using it yet. We've made good progress getting everyone up to speed on git in a relatively short time but there's still some finer points to learn. Thanks for your patience.

PS You described Drupal's patch-based system as if it were working well but in my experience it isn't - patches are not usually applied by the person who wrote them and the person who does the applying doesn't always remember to set attribution. If they do remember they are punished by how laborious it is to do so. I like the PR system better

[mallezie]

I absolutely understand that the change to github is still young, absolutely no offense or worries here. I was just looking for what would be a 'correct' flow, and i'm very glad and appreciating you took the time and willingness to answer. Thanks!

I was wrong on my public PR theory, i got confused by the backdrop approach of github, they set up a public project to use the issue-tracker of github, but that's not the problem / solution here. (Since we use jira for that).

I think you're suggested workflow is correct. This solves the issue of creating new PR's without the initial changes (and attributions). It's not always necessary, mostly the PR-initiator applies changes requested in it's PR. This doesn't however gives the attribution to the commenter on the PR, but that's sort of an other problem, and it is a part / limitation of the github flow. Github isn't exactly designed to work with multiple contributors on one PR. It's off course something to keep in mind when you're further developing some other persons PR.

For the drupal-project I'm not going in the PR vs patches discussion ;-) I have no preference on what's best, that's something i'll let the project decide. Both have advantages, and disadvantages. The attribution giving with the patches is indeed tedious, dreditor could generate however the commit message with attribution for you, and recently this got build in drupal.org. For example on the bottom of an issue (https://www.drupal.org/node/2403243 for example) there is a fieldset 'credit and commiting, which generates an example commit message with the attributions.
https://www.drupal.org/node/2403243

[Coleman Watts]

Cool. For clarification (not sure if I explained this well enough) the new PR would contain your original commits in addition to my new ones. I've updated my answer to clarify, sorry if that was already obvious to you.

And Drupal can just keep on doing it "the Drupal way" if they want to. But I'm really enjoying GitHub

[mallezie]

That's indeed a great solution. Keeping my commit's and adding yours on top.

Let's indeed please stay on github, unless we would build our own better system, but i think there are indeed better uses for the resources. I was just referring to them since i know ther flow, and let's steal their attribution attention (at least in the core project); Thanks for the responses. Is it necessary to document this flow somewhere, if you give me some direction, i'll place it on the wiki or somewhere else.