🗒 Adding notes about providing code changes and pull requests. (Stogl…

…Robotics#95)

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 <https://github.com/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:
 

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 <uc-new-package>` script.
+  .. note:: it is recommended to setup your package using :ref:`setup-new-package <uc-new-package>` script.
 
 The scripts copies template files from the ``templates/robot_bringup`` folder, rename the files, and replaces the placeholders.
 

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 <uc-new-package>` script.
+  .. note:: it is recommended to setup your package using :ref:`create-new-package <uc-new-package>` script.
 
 The scripts copies template files from the ``templates/robot_description`` folder, rename the files, and replaces the placeholders.