From b3aed2c13acd643d637451a34e703f3fdc10b622 Mon Sep 17 00:00:00 2001 From: ll7 <32880741+ll7@users.noreply.github.com> Date: Thu, 24 Oct 2024 12:37:04 +0200 Subject: [PATCH] Update Project Management and PR (#351) * Update project management and review guideline documentation Fixes Update Project Management and PR #350 * Update project management and review guideline documentation * refactor project management and review guideline documentation --- doc/development/project_management.md | 86 +++++------------------- doc/development/review_guideline.md | 95 +++++++++++++++++++-------- 2 files changed, 85 insertions(+), 96 deletions(-) diff --git a/doc/development/project_management.md b/doc/development/project_management.md index 6ac08813..29a1a0f0 100644 --- a/doc/development/project_management.md +++ b/doc/development/project_management.md @@ -1,85 +1,31 @@ # 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" +By clicking "New issue" in the overview you have different templates to create the issue. -Currently, the vehicle can't make u-turns. -Implementing the ability to perform u-turns would be a new feature. +## 2. Create a Pull Request -![feature template](../assets/feature_template.png) - -### πŸš— 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) + -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. +Merge the pull request after: -## Deadlines for pull requests and reviews +1. The review process is complete +2. All reviewer feedback has been addressed +3. All required checks have passed +4. The branch is up-to-date with the main branch -The deadline for submitting a pull request is **Tuesday** before the end of the sprint at **12:15**. +After merging, remember to delete the source branch to keep the repository clean. -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). diff --git a/doc/development/review_guideline.md b/doc/development/review_guideline.md index 005a8650..840adb48 100644 --- a/doc/development/review_guideline.md +++ b/doc/development/review_guideline.md @@ -1,21 +1,23 @@ # 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. Pre-merge Checklist](#51-pre-merge-checklist) + - [5.2. Required Checks](#52-required-checks) + - [5.3. Deleting the branch](#53-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) @@ -38,7 +40,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. @@ -50,15 +54,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?”) @@ -67,7 +73,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. @@ -79,19 +85,56 @@ 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 + +### 5.1. Pre-merge Checklist + +Before merging, ensure: + +- [ ] All conversations are resolved +- [ ] Required CI/CD checks are passing +- [ ] No pending change requests +- [ ] Code has been reviewed thoroughly +- [ ] Documentation is up-to-date + +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.2. 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.3. 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