[Notes] [Git][BuildGrid/buildgrid][laurence/update-contributing.rst] Update CONTRIBUTING.rst



Title: GitLab

Laurence Urhegyi pushed to branch laurence/update-contributing.rst at BuildGrid / buildgrid

Commits:

1 changed file:

Changes:

  • CONTRIBUTING.rst
    ... ... @@ -32,40 +32,31 @@ side effects and quirks the feature may have introduced. More on this below in
    32 32
     
    
    33 33
     .. _BuildGrid mailing list: https://lists.buildgrid.build/cgi-bin/mailman/listinfo/buildgrid
    
    34 34
     
    
    35
    -
    
    36 35
     .. _patch-submissions:
    
    37 36
     
    
    38 37
     Patch submissions
    
    39 38
     -----------------
    
    40 39
     
    
    41
    -We are running `trunk based development`_. The idea behind this is that merge
    
    42
    -requests to the trunk will be small and made often, thus making the review and
    
    43
    -merge process as fast as possible. We do not want to end up with a huge backlog
    
    44
    -of outstanding merge requests. If possible, it is preferred that merge requests
    
    45
    -address specific points and clearly outline what problem they are solving.
    
    46
    -
    
    47 40
     Branches must be submitted as merge requests (MR) on GitLab and should be
    
    48 41
     associated with an issue, whenever possible. If it's a small change, we'll
    
    49 42
     accept an MR without it being associated to an issue, but generally we prefer an
    
    50
    -issue to be raised in advance. This is so that we can track the work that is
    
    43
    +issue to be raised in advance so that we can track the work that is
    
    51 44
     currently in progress on the project.
    
    52 45
     
    
    46
    +When submitting a merge request, please obtain a review from another committer 
    
    47
    +who is familiar with the area of the code base which the branch effects. An 
    
    48
    +approval from another committer who is not the patch author will be needed 
    
    49
    +before any merge (we use Gitlab's 'approval' feature for this).
    
    50
    +
    
    53 51
     Below is a list of good patch submission good practice:
    
    54 52
     
    
    55 53
     - Each commit should address a specific issue number in the commit message. This
    
    56 54
       is really important for provenance reasons.
    
    57
    -- Merge requests that are not yet ready for review must be prefixed with the
    
    58
    -  ``WIP:`` identifier, but if we stick to trunk based development then the
    
    59
    -  ``WIP:`` identifier will not stay around for very long on a merge request.
    
    60
    -- When a merge request is ready for review, please find someone willing to do
    
    61
    -  the review (ideally a maintainer) and assign them the MR, leaving a comment
    
    62
    -  asking for their review.
    
    55
    +- Merge requests that are not yet ready for review should be prefixed with the
    
    56
    +  ``WIP:`` identifier.
    
    63 57
     - Submitted branches should not contain a history of work done.
    
    64 58
     - Unit tests should be a separate commit.
    
    65 59
     
    
    66
    -.. _trunk based development: https://trunkbaseddevelopment.com
    
    67
    -
    
    68
    -
    
    69 60
     Commit messages
    
    70 61
     ~~~~~~~~~~~~~~~
    
    71 62
     
    
    ... ... @@ -89,6 +80,63 @@ For more tips, please read `The seven rules of a great Git commit message`_.
    89 80
     
    
    90 81
     .. _The seven rules of a great Git commit message: https://chris.beams.io/posts/git-commit/#seven-rules
    
    91 82
     
    
    83
    +.. _committer-access:
    
    84
    +
    
    85
    +Committer access
    
    86
    +----------------
    
    87
    +
    
    88
    +In the beginning of the project, we handed out commit access to anyone who had 
    
    89
    +successfully landed a single patch to the code base. We then saw the policy in 
    
    90
    +place over at the `Subversion`_ project and thought this was more sensible, so 
    
    91
    +we decided to employ a very similar - although more light-weight - policy.  
    
    92
    +
    
    93
    +Committers in the BuildGrid project are those folks to whom the right to 
    
    94
    +directly commit changes to our version controlled resources has been granted. 
    
    95
    +The project is meritocratic, which means (among other things) that project 
    
    96
    +governance is handled by those who do the work. While every contribution is 
    
    97
    +valued regardless of its source, not every person who contributes code to the 
    
    98
    +project will earn commit access. The `COMMITTERS`_ file lists all committers.
    
    99
    +
    
    100
    +.. _COMMITTERS: https://gitlab.com/BuildGrid/buildgrid/blob/master/COMMITTERS.md
    
    101
    +.. _Subversion: http://subversion.apache.org/docs/community-guide/roles.html#committers
    
    102
    +
    
    103
    +
    
    104
    +How commit access is granted
    
    105
    +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    
    106
    +
    
    107
    +After someone has successfully contributed a few non-trivial patches, some full 
    
    108
    +committer, usually whoever has reviewed and applied the most patches from that 
    
    109
    +contributor, proposes them for commit access. This proposal is sent only to the 
    
    110
    +other full committers -- the ensuing discussion is private, so that everyone can 
    
    111
    +feel comfortable speaking their minds. Assuming there are no objections, the 
    
    112
    +contributor is granted commit access. The decision is made by consensus; there 
    
    113
    +are no formal rules governing the procedure, though generally if someone strongly 
    
    114
    +objects the access is not offered, or is offered on a provisional basis.
    
    115
    +
    
    116
    +This of course relies on contributors being responsive and showing willingness 
    
    117
    +to address any problems that may arise after landing patches. However, the primary 
    
    118
    +criterion for commit access is good judgment.
    
    119
    +
    
    120
    +You do not have to be a technical wizard, or demonstrate deep knowledge of the 
    
    121
    +entire codebase to become a committer. You just need to know what you don't 
    
    122
    +know. If your patches adhere to the guidelines in this file, adhere to all the usual 
    
    123
    +unquantifiable rules of coding (code should be readable, robust, maintainable, etc.), 
    
    124
    +and respect the Hippocratic Principle of "first, do no harm", then you will probably 
    
    125
    +get commit access pretty quickly. The size, complexity, and quantity of your patches 
    
    126
    +do not matter as much as the degree of care you show in avoiding bugs and minimizing 
    
    127
    +unnecessary impact on the rest of the code. Many full committers are people who have 
    
    128
    +not made major code contributions, but rather lots of small, clean fixes, each of 
    
    129
    +which was an unambiguous improvement to the code. (Of course, this does not mean the 
    
    130
    +project needs a bunch of very trivial patches whose only purpose is to gain commit 
    
    131
    +access; knowing what's worth a patch post and what's not is part of showing good 
    
    132
    +judgement :-) .)
    
    133
    +
    
    134
    +When submitting a merge request, please obtain a review from another committer 
    
    135
    +who is familiar with the area of the code base which the branch effects. Asking on 
    
    136
    +slack is probably the best way to go about this. An approval from a committer 
    
    137
    +who is not the patch author will be needed before any merge (we use Gitlab's 'approval' 
    
    138
    +feature for this).
    
    139
    +
    
    92 140
     
    
    93 141
     .. _coding-style:
    
    94 142
     
    
    ... ... @@ -198,35 +246,6 @@ trunk.
    198 246
     
    
    199 247
     .. _coverage report: https://buildgrid.gitlab.io/buildgrid/coverage/
    
    200 248
     
    
    201
    -
    
    202
    -.. _committer-access:
    
    203
    -
    
    204
    -Committer access
    
    205
    -----------------
    
    206
    -
    
    207
    -We'll hand out commit access to anyone who has successfully landed a single
    
    208
    -patch to the code base. Please request this via Slack or the mailing list.
    
    209
    -
    
    210
    -This of course relies on contributors being responsive and showing willingness 
    
    211
    -to address any problems that may arise after landing branches.
    
    212
    -
    
    213
    -When submitting a merge request, please obtain a review from another committer 
    
    214
    -who is familiar with the area of the code base which the branch effects. An 
    
    215
    -approval from another committer who is not the patch author will be needed 
    
    216
    -before any merge (we use gitlab's 'approval' feature for this).
    
    217
    -
    
    218
    -What we are expecting of committers here in general is basically to escalate the
    
    219
    -review in cases of uncertainty.
    
    220
    -
    
    221
    -.. note::
    
    222
    -
    
    223
    -   We don't have any detailed policy for "bad actors", but will of course handle
    
    224
    -   things on a case by case basis - commit access should not result in commit
    
    225
    -   wars or be used as a tool to subvert the project when disagreements arise.
    
    226
    -   Such incidents (if any) would surely lead to temporary suspension of commit
    
    227
    -   rights.
    
    228
    -
    
    229
    -
    
    230 249
     .. _gitlab-features:
    
    231 250
     
    
    232 251
     GitLab features
    



  • [Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]