Skip to content

Commit

Permalink
Update project management and review guideline documentation
Browse files Browse the repository at this point in the history 
Fixes Update Project Management and PR #350
  • Loading branch information
@ll7
ll7 committed Oct 24, 2024
1 parent 3ce8ec6 commit fa21ce5
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 97 deletions.
81 changes: 10 additions & 71 deletions doc/development/project_management.md
View file
Original file line number Diff line number Diff line change
@@ -1,85 +1,24 @@
# Project management

(Kept from previous group [paf22])
**Summary:** We use [issues](https://github.com/una-auxme/paf/issues) and [milestones](https://github.com/una-auxme/paf/milestones) to manage the project. This document explains how to create issues and pull requests.

**Summary:** We use a [Github Project](https://github.com/users/ll7/projects/2) for project management.
Any bugs or features requests are managed in Github.
- [1. Create bug or feature requests](#1-create-bug-or-feature-requests)
- [2. Create a Pull Request](#2-create-a-pull-request)

- [Project management](#project-management)
- [Create bug or feature requests](#create-bug-or-feature-requests)
- [🐞 Bug](#-bug)
- [Example for "Bug"](#example-for-bug)
- [💡 Feature](#-feature)
- [Example for "Feature"](#example-for-feature)
- [🚗 Bug in CARLA Simulator](#-bug-in-carla-simulator)
- [Example for "Bug in CARLA Simulator"](#example-for-bug-in-carla-simulator)
- [Create a Pull Request](#create-a-pull-request)
- [Merging a Pull Request](#merging-a-pull-request)
- [Deadlines for pull requests and reviews](#deadlines-for-pull-requests-and-reviews)
## 1. Create bug or feature requests

## Create bug or feature requests

Bugs or features can be added [here](https://github.com/ll7/paf22/issues/new/choose) or via the [issue overview](https://github.com/ll7/paf22/issues).
Issues can be added via the [issues overview](https://github.com/una-auxme/paf/issues).

![create issue](../assets/create_issue.png)

By clicking "New issue" in the overview or using the direct link above a wizard guides you to the creation of an issue:

![issue wizard](../assets/issue_wizard.png)

The possibilities are described in the following sections.

### 🐞 Bug

Use this issue type if you encounter a problem which should already work.
If something is not expected to work, but you want to have it, please refer to the Feature section.

#### Example for "Bug"

The documentation says that the vehicle should detect about 90% of the traffic lights.
However, for you it ignores almost all traffic lights.

![bug template](../assets/bug_template.png)

### 💡 Feature

Use this template if you want a new Feature which is not implemented yet.

#### Example for "Feature"

Currently, the vehicle can't make u-turns.
Implementing the ability to perform u-turns would be a new feature.
By clicking "New issue" in the overview you have different templates to create the issue.

![feature template](../assets/feature_template.png)
## 2. Create a Pull Request

### 🚗 Bug in CARLA Simulator

This is a shortcut to directly report an issue in CARLA Simulator which is not an error in this project.

#### Example for "Bug in CARLA Simulator"

CARLA simulator crashes on startup on your machine.

## Create a Pull Request

To create a pull request, go to the [branches overview](https://github.com/ll7/paf22/branches) and select ``New Pull Request`` for the branch you want to create a PR for.
To create a pull request, go to the [branches overview](https://github.com/una-auxme/paf/branches) and select ``New Pull Request`` for the branch you want to create a PR for.
![img.png](../assets/branch_overview.png)
<!-- TODO image is outdated -->

Merge the pull request after the review process is complete and all the feedback from the reviewer has been worked in.

For more information about the review process, see [Review process](./review_guideline.md).

## Merging a Pull Request

The reviewer should always be the person to merge the PR after an approved review.

If the reviewer has anything he/she would like to have changed or clarified, the review should be marked as `Request Changes`.
If there are no uncertainties the reviewer merges the PR. After a revision of the requested changes the reviewer conducts a second review, if he/she is satisfied with the changes, the PR will be merged by him/her.

Long story short, the reviewer who approves the PR should merge. Only approve if there is nothing to change.

## Deadlines for pull requests and reviews

The deadline for submitting a pull request is **Tuesday** before the end of the sprint at **12:15**.

The deadline for submitting a review for a pull request is **Wednesday** before the end of the sprint at **12:15**.
>[!INFO] For more information about the review process, see [Review process](./review_guideline.md).
84 changes: 58 additions & 26 deletions doc/development/review_guideline.md
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
# Review Guidelines

(Kept from previous group [paf22])

**Summary:** This page gives an overview over the steps that should be taken during a review and how to give a helpful and constructive review

- [Review Guidelines](#review-guidelines)
- [How to review](#how-to-review)
- [How to comment on a pull request](#how-to-comment-on-a-pull-request)
- [CodeRabbit](#coderabbit)
- [Incorporating feedback](#incorporating-feedback)
- [Responding to comments](#responding-to-comments)
- [Applying suggested changes](#applying-suggested-changes)
- [Re-requesting a review](#re-requesting-a-review)
- [Resolving conversations](#resolving-conversations)
- [Sources](#sources)

## How to review
**Summary:** This page gives an overview over the steps that should be taken during a Pull-Request review and how to give a helpful and constructive review.

- [1. How to review](#1-how-to-review)
- [2. How to comment on a pull request](#2-how-to-comment-on-a-pull-request)
- [3. CodeRabbit](#3-coderabbit)
- [4. Incorporating feedback](#4-incorporating-feedback)
- [4.1. Responding to comments](#41-responding-to-comments)
- [4.2. Applying suggested changes](#42-applying-suggested-changes)
- [4.3. Re-requesting a review](#43-re-requesting-a-review)
- [4.4. Resolving conversations](#44-resolving-conversations)
- [5. Merging a Pull Request](#5-merging-a-pull-request)
- [5.1. Required Checks](#51-required-checks)
- [5.2. Deleting the branch](#52-deleting-the-branch)
- [6. Deadlines for pull requests and reviews](#6-deadlines-for-pull-requests-and-reviews)
- [7. Sources](#7-sources)

## 1. How to review

1. Select the PR you want to review on GitHub
![img.png](../assets/PR_overview.png)
Expand All @@ -38,7 +39,9 @@
13. Request Changes - this pull request is not mergeable and fixes are needed
11. Click `Submit Review`

## How to comment on a pull request
Most of these steps can be done in the GitHub web interface or in VSCode with the GitHub Pull Request and Issues extension.

## 2. How to comment on a pull request

- Familiarize yourself with the context of the issue, and reasons why this Pull Request exists.
- If you disagree strongly, consider giving it a few minutes before responding; think before you react.
Expand All @@ -50,15 +53,17 @@
- Avoid hyperbole. (“NEVER do…”)
- Aim to develop professional skills, group knowledge and product quality, through group critique.
- Be aware of negative bias with online communication. (If content is neutral, we assume the tone is negative.) Can you use positive language as opposed to neutral?
- Use emoji to clarify tone. Compare “✨ ✨ Looks good 👍 ✨ ✨” to “Looks good.”

## CodeRabbit
## 3. CodeRabbit

The repository also comes with CodeRabbit integration. This tool generates automatic reviews for a pull request. Although the proposed changes do not have to be incorporated, they can point to a better solution for parts of the implementation.
The repository also comes with CodeRabbit integration.
This tool generates automatic reviews for a pull request.
Although the proposed changes do not have to be incorporated, they can point to a better solution for parts of the implementation.
CodeRabbit is not considered to be a sufficient review, but it can be a helpful addition to the standard review process.

## Incorporating feedback
## 4. Incorporating feedback

### Responding to comments
### 4.1. Responding to comments

- Consider leading with an expression of appreciation, especially when feedback has been mixed.
- Ask for clarification. (“I don’t understand, can you clarify?”)
Expand All @@ -67,7 +72,7 @@ The repository also comes with CodeRabbit integration. This tool generates autom
- Link to any follow up commits or Pull Requests. (“Good call! Done in 1682851”)
- If there is growing confusion or debate, ask yourself if the written word is still the best form of communication. Talk (virtually) face-to-face, then mutually consider posting a follow-up to summarize any offline discussion (useful for others who be following along, now or later).

### Applying suggested changes
### 4.2. Applying suggested changes

If the reviewer not only left comments but also made specific suggestions on code parts, as shown in [How to review](#how-to-review), you can incorporate them straight away.

Expand All @@ -79,19 +84,46 @@ If the reviewer not only left comments but also made specific suggestions on cod
5. In the commit message field, type a short and meaningful commit message according to the [commit rules](./commit.md)
6. Click ``Commit changes``

### Re-requesting a review
### 4.3. Re-requesting a review

If you made substantial changes to your pull request and want to a fresh review from a reviewer, contact him directly. It is appropriate to ask the same reviewer from the initial pull request as he/she is most familiar with the pull request.

### Resolving conversations
### 4.4. Resolving conversations

If a comment of a review was resolved by either, a new commit or a discussion between the reviewer and the team that created the pull request, the conversation can be marked as resolved by clicking ``Resolve conversation`` in the ``Conversation`` or ``Files Changed`` tab of the pull request on GitHub.
If a new commit took place it is encouraged to comment the commit SHA to have a connection between comment and resolving commit
![img.png](../assets/Resolve_conversation.png)

> [!INFO] All conversations should be resolved before merging the pull request.
## 5. Merging a Pull Request

The reviewer should always be the person to merge the PR after an approved review.

If the reviewer has anything he/she would like to have changed or clarified, the review should be marked as `Request Changes`.
If there are no uncertainties the reviewer merges the PR. After a revision of the requested changes the reviewer conducts a second review, if he/she is satisfied with the changes, the PR will be merged by him/her.

Long story short, the reviewer who approves the PR should merge. Only approve if there is nothing to change.

### 5.1. Required Checks

Before merging a pull request, the request checks by the CI/CD pipeline should be successful. If the checks fail, the pull request should not be merged.

> [!INFO] An exception can be made for a PR that only addresses the documentation and the `driving` check is not yet completed.
### 5.2. Deleting the branch

After the PR is merged, the branch should be deleted. This should be done by the person who merged the PR.

## 6. Deadlines for pull requests and reviews

The deadline for submitting a pull request is **Friday 10:00 am** before the end of the sprint.

The deadline for submitting a review for a pull request is **Monday 10:00 am** before the end of the sprint.

---

## Sources
## 7. Sources

<https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request>

Expand Down

0 comments on commit fa21ce5

Please sign in to comment.