|
| 1 | +.. role:: bolditalic |
| 2 | + :class: bolditalic |
| 3 | + |
| 4 | +======================= |
| 5 | +Contributing guidelines |
| 6 | +======================= |
| 7 | + |
| 8 | +We thank you in advance |thumbup| |tada| for taking the time to |
| 9 | +contribute, whether with *code* or with *ideas*, to the Local EGA |
| 10 | +project. |
| 11 | + |
| 12 | +------------------------ |
| 13 | +AGILE project management |
| 14 | +------------------------ |
| 15 | + |
| 16 | +We use *Zenhub*, the Agile project management within Github. |
| 17 | + |
| 18 | +You should first `install it`_ if you want to contribute or just follow the project progress. |
| 19 | +You can also use the `Zenhub app`_ if you wish. |
| 20 | + |
| 21 | +In short, the `AGILE method`_ helps developers organize themselves: |
| 22 | + |
| 23 | +* They decide about the tasks (not the managers) |
| 24 | +* Main Tasks should be divided into smaller manageable ones. The big |
| 25 | + tasks are called *Epics*. |
| 26 | +* We have a given period (called Sprint) to work on a chosen |
| 27 | + task. Here, a Sprint spans across 2 weeks. |
| 28 | +* We review the work done at the end of the Sprint, closing issues or |
| 29 | + pushing them into the next Sprint. Ideally, they are sub-divided in |
| 30 | + case they encounter obstacles. |
| 31 | +* We have a short meeting every weekday at 9:30 AM. We call it a |
| 32 | + *standup* and we use it to keep everyone on point, and identify |
| 33 | + quickly blockers. It's not a lengthy discussion. We ask: |
| 34 | + |
| 35 | + - What did you get done yesterday (or last week, last month, etc.)? |
| 36 | + - What are you working on now? |
| 37 | + - What isn’t going well, and on what could you use help? |
| 38 | + |
| 39 | +--------- |
| 40 | +Procedure |
| 41 | +--------- |
| 42 | + |
| 43 | +1. Create an issue on Github, and talk to the team members on the NBIS |
| 44 | + local-ega Slack channel. You can alternatively pick one already |
| 45 | + created. |
| 46 | + |
| 47 | +.. note:: |
| 48 | + Contact `Jonas Hagberg`_ to request access if you are not part of that channel already. |
| 49 | + |
| 50 | + |
| 51 | +2. Assign yourself to that issue. |
| 52 | + |
| 53 | +#. Discussions on how to proceed about that issue take place in the |
| 54 | + comment section on that issue, beforehand. |
| 55 | + |
| 56 | + The keyword here is *beforehand*. It is usually a good idea to talk |
| 57 | + about it first. Somebody might have already some pieces in place, |
| 58 | + we avoid unnecessary work duplication and a waste of time and |
| 59 | + effort. |
| 60 | + |
| 61 | +#. Work on it (on a fork, or on a separate branch) as you wish. That's |
| 62 | + what ``git`` is good for. This GitHub repository follows |
| 63 | + the `coding guidelines from NBIS`_. |
| 64 | + |
| 65 | + Name your branch as you wish and prefix the name with: |
| 66 | + |
| 67 | + * ``feature/`` if it is a code feature |
| 68 | + * ``hotfix/`` if you are fixing an urgent bug |
| 69 | + |
| 70 | + Use comments in your code, choose variable and function names that |
| 71 | + clearly show what you intend to implement. |
| 72 | + |
| 73 | + Use `git rebase -i`_ in |
| 74 | + order to rewrite your history, and have meaningful commits. That |
| 75 | + way, we avoid the 'Typo', 'Work In Progress (WIP)', or |
| 76 | + 'Oops...forgot this or that' commits. |
| 77 | + |
| 78 | + Limit the first line of your git commits to 72 characters or less. |
| 79 | + |
| 80 | + |
| 81 | +#. Create a Pull Request (PR), so that your code is reviewed by the |
| 82 | + admins on this repository. |
| 83 | + |
| 84 | + That PR should be connected to the issue you are working on. |
| 85 | + Moreover, the PR: |
| 86 | + |
| 87 | + - should use ``Estimate=1``, |
| 88 | + - should be connected to: |
| 89 | + |
| 90 | + * an ``Epic``, |
| 91 | + * a ``Milestone`` and |
| 92 | + * a ``User story`` |
| 93 | + * ... or several. |
| 94 | + |
| 95 | + N.B: Pull requests are done to the ``dev`` branch. PRs to ``master`` are rejected. |
| 96 | + |
| 97 | +#. Selecting a review goes as follows: Pick one *main* reviewer. It |
| 98 | + is usually one that you had discussions with, and is somehow |
| 99 | + connected to that issue. If this is not the case, pick several reviewers. |
| 100 | + |
| 101 | + Note that, in turn, the main reviewer might ask another reviewer |
| 102 | + for help. The approval of all reviewers is compulsory in order to |
| 103 | + merge the PR. Moreover, the main reviewer is the one merging the |
| 104 | + PR, not you. |
| 105 | + |
| 106 | + Find more information on the `NBIS reviewing guidelines`_. |
| 107 | + |
| 108 | + |
| 109 | +#. It is possible that your PR requires changes (because it creates |
| 110 | + conflicts, doesn't pass the integrated tests or because some parts |
| 111 | + should be rewritten in a cleaner manner, or because it does not |
| 112 | + follow the standards, or you're requesting the wrong branch to pull |
| 113 | + your code, etc...) In that case, a reviewer will request changes |
| 114 | + and describe them in the comment section of the PR. |
| 115 | + |
| 116 | + You then update your branch with new commits. We will see the PR |
| 117 | + changes faster if you ping the reviewer in the slack channel. |
| 118 | + |
| 119 | + Note that the comments *in the PR* are not used to discuss the |
| 120 | + *how* and *why* of that issue. These discussions are not about the |
| 121 | + issue itself but about *a solution* to that issue. |
| 122 | + |
| 123 | + Recall that discussions about the issue are good and prevent |
| 124 | + duplicated or wasted efforts, but they take place in the comment |
| 125 | + section of the related issue (see point 4), not in the PR. |
| 126 | + |
| 127 | + Essentially, we don't want to open discussions when the work is |
| 128 | + done, and there is no recourse, such that it's either accept or |
| 129 | + reject. We think we can do better than that, and introduce a finer |
| 130 | + grained acceptance, by involving *beforehand* discussions so that |
| 131 | + everyone is on point. |
| 132 | + |
| 133 | + |
| 134 | + |
| 135 | +------------------- |
| 136 | +Did you find a bug? |
| 137 | +------------------- |
| 138 | + |
| 139 | +* Ensure that the bug was not already reported by `searching under Issues`_. |
| 140 | + |
| 141 | +* Do :bolditalic:`not` file it as a plain GitHub issue (we use the |
| 142 | + issue system for our internal tasks (see Zenhub)). If you're unable |
| 143 | + to find an (open) issue addressing the problem, `open a new one`_. |
| 144 | + Be sure to prefix the issue title with **[BUG]** and to include: |
| 145 | + |
| 146 | + - a *clear* description, |
| 147 | + - as much relevant information as possible, and |
| 148 | + - a *code sample* or an (executable) *test case* demonstrating the expected behaviour that is not occurring. |
| 149 | + |
| 150 | +* If possible, use the following `template to report a bug`_. |
| 151 | + |
| 152 | +.. todo:: Make that template |
| 153 | + |
| 154 | + |
| 155 | +---- |
| 156 | + |
| 157 | +| Thanks again, |
| 158 | +| /NBIS System Developers |
| 159 | +
|
| 160 | +.. _Zenhub: https://www.zenhub.com |
| 161 | +.. _install it: https://www.zenhub.com/extension |
| 162 | +.. _Zenhub app: https://app.zenhub.com |
| 163 | +.. _AGILE method: https://www.zenhub.com/blog/how-to-use-github-agile-project-management |
| 164 | +.. _Jonas Hagberg: https://nbis.se/about/staff/jonas-hagberg/ |
| 165 | +.. _coding guidelines from NBIS: https://github.com/NBISweden/development-guidelines |
| 166 | +.. _git rebase -i: https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History |
| 167 | +.. _NBIS reviewing guidelines: https://github.com/NBISweden/development-guidelines#how-we-do-code-reviews |
| 168 | +.. _searching under Issues: https://github.com/NBISweden/LocalEGA/issues?utf8=%E2%9C%93&q=is%3Aissue%20label%3Abug%20%5BBUG%5D%20in%3Atitle |
| 169 | +.. _open a new one: https://github.com/NBISweden/LocalEGA/issues/new?title=%5BBUG%5D |
| 170 | +.. _template to report a bug: todo |
| 171 | +.. |tada| unicode:: U+1f389 |
| 172 | +.. |thumbup| unicode:: U+1f44d |
| 173 | + |
0 commit comments