From 5efbf30babc552d59f328f360c5aa5be542847b9 Mon Sep 17 00:00:00 2001 From: "Dr. Denis" Date: Fri, 20 Jan 2023 18:33:26 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=92=20Adding=20notes=20about=20providi?= =?UTF-8?q?ng=20code=20changes=20and=20pull=20requests.=20(#95)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/guidelines/development.rst | 59 ++++++++++++++++++- .../setup_robot_bringup_package.rst | 2 +- .../setup_robot_description_package.rst | 2 +- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/docs/guidelines/development.rst b/docs/guidelines/development.rst index 1d40eaca..0721fc1d 100644 --- a/docs/guidelines/development.rst +++ b/docs/guidelines/development.rst @@ -7,15 +7,68 @@ This documents proposes development guidelines to increase work efficiency and s **NOTE**: All the proposal here are the results of authors' personal experiences. Saying that, if you have any idea to make them better you are very wellcome to create a PR. +Overview +========= -1. Use *pre-commit* for formatting and linting -=============================================== +1. `Making changes to the code-base by submitting a Pull Request <#>`_ + +2. `Using *pre-commit* <#use-pre-commit-for-formatting-and-linting>`_ + + +Code Changes and Pull Request Submissions +========================================== + +To make a change of the existing code base, Pull Request (GitHub) or Merge Request (GitLab) are used. +This section describes in short process with valuable tips to make your and reviewers' life easier. + +#. Check with the team if it is usual to submit a PR/MR from a fork or directly into target repository. + This usually depends on the team size and organization. + When working with public repositories you always need to create a fork. + + .. note:: + + In *Stogl Robotics* we have the organization called `StoglRobotics-forks `_ where all forks of public repositories live and are accessible for writing by all team members. + This simplify collaboration inside the team - there is no need for individual access grants when using forks under your user. + **Always** check if there is already a fork in *StoglRobotics-forks* organization and if not create it. + +#. Start development always from the up-to date state of the repositories default branch (usually called *master* or *main* - for simplicity we call it *master* here). + Take into account that master branch of your fork is usually not up to date with the upstream repository. + Therefore be careful about that and use the opportunity to sync *master* branch of the fork to the state of the *master* branch of the upstream repository. + +#. **Always** create a new branch for each feature or bug fix. Don't submit PRs from *master* branch. + +#. **Before** starting development check how the branch will be merged, using *merge commit* or *squash* method. + If *merge commit* is used make sure that each of your commits is clean and named properly since they will become part of the repository's history. + + .. note:: + + In *Stogl Robotics* we are always *squashing* commits, i.e., one future or one bug fix is one commit in the default branch. The commit message is edited before merging. + +#. Explain in the PR/MR description what your code is doing and why. + +#. When you are finished with development and want to submit code for the review - consider the following tips: + + - **Always** run *pre-commit* formaters; + - Review your code **first by yourself** before asking someone else; + - Make sure there **are not** commented code blocks or if they have to be there add explanation why; + - Resolve all TODOs or add concrete questions about them either in the code or in review comments so that other people know this is open for discussion; + - By iterating on the review adjust **all parts** of the code with the same or similar patterns even if you get a comment about those only in one place - reviewers usually don't like to repeat themselves on each iteration of the same issue - if you are not sure about something, ask; + - Ask yourself: *Would I like to review this code?*. + + .. important:: + + The reviews are done by other people and show respect toward their time. Reviewer's task is **not to clean** the code behind you. + Many people get very angry if you provide messy code, usually so much that you have to wait for weeks to get your code reviewed again. + + +Use *pre-commit* for formatting and linting +============================================ ``pre-commit`` is a program that adds hooks into ``git`` so when you commit something actions can be automatically executed. There are many different possibility with ``pre-commit`` but it is mostly used for integrating code linters and formatters to always commit clean code. Getting started with *pre-commit* ------------------------------------- +---------------------------------- First install it to you computer using: diff --git a/docs/use-cases/ros_packages/setup_robot_bringup_package.rst b/docs/use-cases/ros_packages/setup_robot_bringup_package.rst index bed7b542..f3de86bb 100644 --- a/docs/use-cases/ros_packages/setup_robot_bringup_package.rst +++ b/docs/use-cases/ros_packages/setup_robot_bringup_package.rst @@ -14,7 +14,7 @@ Script for Setting up Description Package If the package name is not set, it is guessed from the current path using the folder's name. The script **has to be executed** from the folder where the bringup should be generated. -**Note**: it is recommended to setup your package using :ref:`setup-new-package ` script. + .. note:: it is recommended to setup your package using :ref:`setup-new-package ` script. The scripts copies template files from the ``templates/robot_bringup`` folder, rename the files, and replaces the placeholders. diff --git a/docs/use-cases/ros_packages/setup_robot_description_package.rst b/docs/use-cases/ros_packages/setup_robot_description_package.rst index 6f830c14..04da7e76 100644 --- a/docs/use-cases/ros_packages/setup_robot_description_package.rst +++ b/docs/use-cases/ros_packages/setup_robot_description_package.rst @@ -14,7 +14,7 @@ Script for Setting up Description Package If the package name is not set, it is guessed from the current path using the folder's name. The script **has to be executed** from the package folder where the description should be generated. -**Note**: it is recommended to setup your package using :ref:`create-new-package ` script. + .. note:: it is recommended to setup your package using :ref:`create-new-package ` script. The scripts copies template files from the ``templates/robot_description`` folder, rename the files, and replaces the placeholders.