Commit ecc0afcf authored by Dorota Czaplejewicz's avatar Dorota Czaplejewicz

contributing: Update mistakes from c80241cf

The changes were merged prematurely. This commits takes into account remaining feedback from !33
parent c80241cf
Pipeline #174 passed with stage
in 41 seconds
......@@ -10,7 +10,7 @@ Aside from project specific issues, we do have an `Apps_issues <https://source.p
Working on issues
*****************
Issues have an *Assignee* field, which is used for indicating that someone is working on it. Other people can avoid doing the same work if it's already being taken care of. Often, it will be the same person who filed the issue, but anyone interested can assign themselves.
Issues have an *Assignee* field, which is used for indicating that someone is working on it. Other people can avoid doing the same work if it's already being taken care of. Anyone interested can assign themselves, even the person who filed the issue.
If you are filing an issue, and you know someone else can solve it better than you can, please use the ``@mention`` to bring their attention. Don't assign other people yourself! Let them take a look before taking the responsibility.
......@@ -43,7 +43,7 @@ Making Merge Requests
Merge requests (MRs) are meant for code which you would like to get merged. There are two possible types of those:
* *Work In Progress* requests have the "WIP: " prefix in the title and are meant to gather feedback on the idea. These MRs won't necessarily be tested, and will not be merged. The following guidelines don't apply to them. However, the author may remove the "WIP: " prefix, making it a regular merge request.
* *Work In Progress* requests have the "WIP: " prefix in the title and are meant to gather feedback on the idea. These MRs can have failing pipelines, don't need to be tested, and will not be merged. The following guidelines don't apply to them. However, the author may remove the "WIP: " prefix, making it a regular merge request.
* Regular merge requests are a way for the author to take a commitment to work on the code. In exchange, the project maintainers will provide feedback and include the change when they think it's appropriate.
......@@ -51,27 +51,37 @@ By making a merge request you are letting others know that your change has been
* In the merge request, add a sign off line signifying that you agree that your work is used under the license of the project being contributed to: ``Signed-off-by: Random J Developer <random@developer.example.org>``
* Anyone that is making a MR should have tested their change.
* Alert someone to review the MR, ideally by using a ``@mention``.
* If there is no single person that knows the affected areas well, alert multiple people.
* Alert one of the maintainers to review the MR, ideally by using a ``@mention``. Maintainers are listed in the DOAP file.
* If there is no single maintainer that knows the affected areas well, alert multiple ones.
.. note:: If you have a MR that depends on another outstanding MR (or if you only want feedback without a merge), prepend your MR title with "WIP". For more info on WIP, see `gitlab's WIP documentation <https://docs.gitlab.com/ee/user/project/merge_requests/work_in_progress_merge_requests.html>`_
Reviewing Merge Requests
************************
*Work In Progress* merge requests serve only as a place for discussion, therefore no special procedure is required. For all other merge requests, please adhere to the following guidelines.
*Work In Progress* merge requests serve only as a place for discussion, therefore no special procedure is required. A MR is valid even if the CI pipeline fails. For all other merge requests, please adhere to the following guidelines.
Merge requests come in different sizes, and depending on the complexity, one or more people may want to review it. However, it's enough that one person tests it.
Anyone can comment and provide feedback on MRs. A special status is reserved for project members, who may become *Reviewers*. *Reviewers* take up the task of testing or providing feedback. It depends on their opinion whether the MR is accepted or not.
If you are listed in the DOAP file as a maintainer, developer, or helper, you can take up the role of a *Reviewer* on a merge request.
First off, save your time by skipping MRs which have a failing CI build. It's the responsibility of the submitter to provide working code.
When multiple maintainers are interested in the outcome of the MR, please let them have an opportunity to speak, by using a ``@mention``. This may happen for example when the MR is the outcome of a discussion or when it modifies areas maintained by multiple persons.
At the same time, it's enough that one person *tests* a merge request.
To succesfully review someone's MR, first indicate that you are reviewing the MR by making yourself the *Assignee*, or by saying that explicitly in the comments. Then:
* Build and test the newest version of the MR if no other reviewer did that yet.
* Build and test the newest version of the MR if no one else did that yet.
* Provide feedback in the MR discussion. This is where you would mention additional changes needed.
Once all reviewers are satisfied with the MR, merge it.
Once all *reviewers* are satisfied with the MR, merge it.
Do NOT merge the request if any of these are true:
* The MR has "WIP" in the title.
* If no reviewer tested the current version of the MR.
* If there are unresolved discussions.
* The CI build of the current revision failed.
* No project member tested the current version of the MR.
* There are unresolved discussions.
* A maintainer was asked to take a look at the MR, but didn't have the chance to speak yet.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment