diff --git a/source/contributor-guide/bug-reporting-structured.rst b/source/contributor-guide/bug-reporting-structured.rst index 4771b46..2427127 100644 --- a/source/contributor-guide/bug-reporting-structured.rst +++ b/source/contributor-guide/bug-reporting-structured.rst @@ -1,150 +1,152 @@ .. _contributor-guide-structured-contributions-reporting-bugs: =========================================================== Reporting Bugs Against (Long-Term) Supported Kolab Versions =========================================================== Long-term support versions of Kolab maintain larger sets of different versions of software, and it becomes important to ensure that the fix for an issue in `foo-1.0` does not require a properly entitled customer to upgrade to a later version of Kolab, or even a later version of `foo`. The stability requirements imply that support be able to track issues with specific versions of the software, targets these issues to be resolved in certain newer versions of the software, and trusts the resolution of the issue to be verifiable against multiple versions of the software collection. Example ======= The following table depicts the versions of the |roundcubemail| package, and the |roundcubemail-plugins-kolab| package, as distributed for each product stream. .. table:: Version table for Roundcubemail and Kolab Plugins +--------------+-----------------+-------------------------------+ | | |roundcubemail| | |roundcubemail-plugins-kolab| | +==============+=================+===============================+ | |KE13| | `1.0.4` | `3.1.16` | +--------------+-----------------+-------------------------------+ | |KE14| | `1.1.4` | `3.2.11` | +--------------+-----------------+-------------------------------+ | |K16| | `1.2` | `3.3` | +--------------+-----------------+-------------------------------+ | |Winterfell| | `1.2` | `3.3` | +--------------+-----------------+-------------------------------+ Stability requirements for |KE13| demand the following: * The version of |roundcubemail| shipped to the product stream is one of the **1.0 series** of upstream releases, -* Upstream maintains a **stable** 1.0 series for roundcubemail, +* Upstream maintains a **stable** 1.0 series for |roundcubemail|, * The version of |roundcubemail-plugins-kolab| shipped to the product stream is one of the **3.1 series** of upstream releases, -* Upstream maintains a **stable** 3.1 series for roundcubemail-plugins-kolab, +* Upstream maintains a **stable** 3.1 series for + |roundcubemail-plugins-kolab|, * Future versions of |roundcubemail| in the 1.0 series remain backward compatibility for consumers of its API (such as |roundcubemail-plugins-kolab|), * Added functionality in |roundcubemail-plugins-kolab|, if any, does not - require any changes in |roundcubemail| that would break other compatibility, + require any changes in |roundcubemail| that would break other + compatibility, * The aforementioned conditions all last for up to 5 years, * For a large number of target platforms. The same conditions apply to |KE14| and |K16| -- with different version series, different timelines and sometimes different stacks. Further down the dependency stack, this includes maintenance for **libkolabxml**, **libkolab** and **libcalendaring**. In the future, this will extend up to 6 product streams that will need to be maintained. It is therefore important that an issue logged against a version 1.0.1, with the current version in the 1.0 series perhaps being 1.0.10, needs to go through the following stages: #. Is the issue reproducible in the unstable development version of the software suite? a. If it is, it is a development issue; `report a bug`_ in Phabricator. Developer teams become responsible for testing the resolution of the issue first, and fixing the issue second (see :ref:`contributor-guide-test-driven-development`). For each of the product streams in between current development and the original issue report, create a ticket in `bugzilla`_. .. seealso:: * Something about backporting with the verification included. b. If it is not, in what product stream can the issue still be reproduced? This process can be a very costly lather-rinse-repeat exercise, and therefore needs to be covered ahead of time, through :ref:`contributor-guide-test-driven-development`. Create a `bugzilla`_ ticket for the appropriate .. seealso:: * Something about constructive troubleshooting efforts #. Using the input of 1) and outcome of 1a) or 1b), describe the range of targets; * branch 1.2 for |K16|: yes or no? * branch 1.1 for |KE14|: yes or no? * etc. Each becomes a separate `bugzilla`_ ticket, each of them depending on the resolution of the more recent product stream('s software version). Example ======= Let's assume "yay" fails on all versions of Roundcube. #. Issue: - *Yay fails on Roundcube 1.0.1 on* :gray:`Enterprise Linux 6` + *Yay fails on Roundcube 1.0.1 on* |santiago| This is an original ticket. The intended milestone for the resolution is `1.0-next`. #. Support: #. *Does yay fail in* |Winterfell| *on* |maipo| *?* Yes, this is a new development issue; `Report a bug`_. Add a project tag for :orange:`Winterfell` #. *Does yay fail in* |K16| *on* |maipo| *?* Yes, and |K16| is on the same software version series as |Winterfell|; It is thus a new development issue; `Report a bug`_ and have it blocked by the bug created in 2.1). Associate the |K16| release target with the bug. Also create a `bugzilla`_ ticket. The milestone here is `1.2-next`. Refer to the task created in Phabricator. #. *Does yay fail in* |KE14| *on* |santiago| *?* Yes, create a `bugzilla`_ ticket and block it with the ticket created in 2.2). The milestone here is `1.1-next`. #. *Does yay fail in* |KE13| *on* |santiago| *using version 1.0.4?* Yes, block the original ticket in 1) with the ticket from 2.3). diff --git a/source/contributor-guide/peer-review.rst b/source/contributor-guide/peer-review.rst index c6f53e2..8fe5271 100644 --- a/source/contributor-guide/peer-review.rst +++ b/source/contributor-guide/peer-review.rst @@ -1,33 +1,150 @@ .. _contributor-guide-peer-review: =========== Peer Review =========== -Peer review is a process in which developers submit code changes to a staging -environment, so that other peers have the opportunity to review the work before -the code changes are submitted back to the source code management repository. +Peer review is a process in which code changes are submitted to a staging +environment, so that fellow developers have the opportunity to review the work +before the code changes are submitted back to the source code management +repository. -In our development platform, this process is facilitated by `Differential`_. To -use `Differential`_, you must install **arcanist** and set it up. +Peer view is assisted by `Differential`_. To use `Differential`_ effectively, +you must install **arcanist** and set it up. .. seealso:: * :ref:`contributor-guide-setup-your-development-environment` -In Kolab development, this stage allows work to progress on writing tests that -fail, first, subsequently supplemented by the work on code changes that make -those tests succeed. +For structured contributions, and for sprint participants, the use of +`Differential`_ is obligatory, and further facilitates the process of +:ref:`contributor-guide-test-driven-development`. -.. seealso:: +Post-poning destabilizing changes stabilizes the day-to-day use of +|Winterfell|, and allows multiple changesets to land at once (enabling a single +feature to span multiple software projects). + +The Process +=========== + +A task in a sprint should consist of a bite-size chunk of development work. + +When you start work on the task, you write the tests first (and those tests +would fail, because they have not been backed by code changes). + +You submit your differential at the earliest opportunity, however incomplete +your coverage may be at that point. This ensures peers can participate in +feedback cycles early, and shows progress being made as it is made. + +Creating a Differential for Review +================================== + +Your development takes place against the **master** branch, unless you find +yourself running in circles, interrupted by a support request. + +Thus, make sure you have **master** checked out, and for the sake of preventing +superfluous merge and rebase exercises, ensure it's in sync with upstream: + +.. parsed-literal:: + + $ :command:`git checkout master` + $ :command:`git fetch origin` + $ :command:`git rebase origin/master --autostash` + +Provided a ticket, such as :task:`1037`, you should branch off the GIT +repository; + +.. parsed-literal:: + + [kanarip\@dws06 docs.git (master u=)]$ :command:`git checkout -b dev/T1037` + [kanarip\@dws06 docs.git (dev/T1037)]$ + +This will allow you to keep your changes out of a tracking working copy, and +allows you to switch back over to other work without stacking the changes on +top of one another. + +Make your changes, and commit them in however many commits you think is +reasonable. + +Then, create the `Differential`_: + +.. parsed-literal:: + + [kanarip\@dws06 docs.git (dev/T1037 %)]$ arc diff + You have untracked files in this working copy. + + Working copy: /home/kanarip/devel/src/kolab/docs.git/ + + Untracked changes in working copy: + (To ignore these changes, add them to ".git/info/exclude".) + quick-notes.txt + + Ignore these untracked files and continue? [y/N] :command:`y` + +You will now be requested to provide some information about your proposed +changes. + +The first line of the differential submission is the title for the +differential. + +The template offers a ``Summary:`` line, and you are to make that line include +the phrase ``Resolves T1037``. This causes the differential to be associated +with the ticket, and will cause the ticket to change status to resolved when +the differential is accepted, merged and pushed back. + +Use the remainder of the space under ``Summary: Resolves T1037`` to spout any +thoughts or comments. + +In ``Test Plan:``, you list the steps needed to verify the work. This could be: + +.. parsed-literal:: + + Test Plan: Run the unittests with; + + $ NOSE_VERBOSE=2 nosetests tests/unit/ + +In ``Reviewers:``, you list the development team associated with the project +(such as :red:`PyKolab Developers` or :red:`Documentation Authors`). You are to +use hashtags here, so they would be ``#pykolab_developers`` or +``#documentation_authors``. **arcanist** will validate the entries and prompt +you if they do not exist. + +In ``Subscribers:``, you may either not list anyone, or list the username(s) of +people you wish to be notified when the differential changes. Here too, +**arcanist** validates the input. + +.. parsed-literal:: + + Linting... + No lint engine configured for this project. + Running unit tests... + No unit test engine is configured for this project. + Updating commit message... + Created a new Differential revision: + Revision URI: https://git.kolab.org/D89 + + Included changes: + M source/index.rst + A source/contributor-guide/index.rst + +Review Process +============== + +A reviewer must code changes are verifiable, and validate unit, functional and +integration tests for the code changes before accepting the differential. + +A reviewer will want these tests to be automated, or ends up spending a lot of +time and effort on verifying the changes made. - * :ref:`contributor-guide-test-driven-development` +Accepting the differential does not mean the code changes are automatically +merged, however. Acceptance of a differential outside of the merge window is +therefore allowed. The changes submitted are reviewed on Thursday afternoons at the latest. Your changes need to be reviewed by at least one other person, who is a software development project member. In :ref:`contributor-guide-test-driven-development`, the submission of the differential associated with your review process aides in the staging of the code changes. diff --git a/source/contributor-guide/setup-development-environment.rst b/source/contributor-guide/setup-development-environment.rst index 4b7e13c..02df9b8 100644 --- a/source/contributor-guide/setup-development-environment.rst +++ b/source/contributor-guide/setup-development-environment.rst @@ -1,256 +1,203 @@ .. _contributor-guide-setup-your-development-environment: ================================== Setup Your Development Environment ================================== This guide sets you up a development workstation to contribute to Kolab Groupware development on `Phabricator`_. #. :ref:`contributor-guide-setup-tools` .. _contributor-guide-setup-tools: Install the Tools ================= #. Ensure you have our Tools repository configured. .. parsed-literal:: # :command:`cd /etc/yum.repos.d/` For **Fedora 23**: .. parsed-literal:: # :command:`wget https://obs.kolabsys.com/repositories/Tools/Fedora_23/Tools.repo` For **Fedora 22**: .. parsed-literal:: # :command:`wget https://obs.kolabsys.com/repositories/Tools/Fedora_22/Tools.repo` For **Red Hat Enterprise Linux 7**: .. parsed-literal:: # :command:`wget https://obs.kolabsys.com/repositories/Tools/RHEL_7/Tools.repo` #. Import the GPG public key the packages are signed with: .. parsed-literal:: # :command:`rpm --import https://ssl.kolabsys.com/community.asc` #. Install **arcanist**: For **Fedora 22** and **Fedora 23**: .. parsed-literal:: # :command:`dnf -y install arcanist` For **Red Hat Enterprise Linux 7**: .. parsed-literal:: # :command:`yum -y install arcanist` +#. Hook up arcanist to Phabricator: + + .. parsed-literal:: + + # :command:`arc set-config default https://git.kolab.org/` + # :command:`arc install-certificate` + + And follow the instructions. + .. _contributor-guide-setup-recommended-configuration: Recommended Configuration ========================= **SSH Configuration** Configure SSH to use the correct username and SSH identity when you use ``git.kolab.org``: .. parsed-literal:: $ :command:`grep -A3 git.kolab.org ~/.ssh/config` Host git.kolab.org User git IdentityFile ~/.ssh/id_rsa **BASH Completion** Say something about bash completion and how great it is. **GIT Prompt** Say something about the git prompt. **GIT Configuration** For each repository separately, or otherwise globally: .. parsed-literal:: $ :command:`git config [--global] user.name "Jeroen van Meeuwen (Kolab Systems)"` $ :command:`git config [--global] user.email vanmeeuwen\@kolabsys.com` $ :command:`git config [--global] branch.autosetuprebase always` $ :command:`git config [--global] push.default matching` **~/.bashrc** A recommended snippet for `~/.bashrc` to assist you visually: .. parsed-literal:: export GIT_PS1_SHOWDIRTYSTATE=1 export GIT_PS1_SHOWUNTRACKEDFILES=1 export GIT_PS1_SHOWUPSTREAM="auto verbose" if [ ! -f "/etc/bash_completion" ]; then if [ -f "/etc/bash_completion.d/git" ]; then cp /etc/bash_completion.d/git ~/.git-completion.sh . ~/.git-completion.sh PS1='[\u\@\h \W$(__git_ps1 " (%s)")]\$ ' elif [ -f "/usr/share/bash-completion/completions/git" ]; then cp /usr/share/bash-completion/completions/git ~/.git-completion.sh . ~/.git-completion.sh PS1='[\u\@\h \W$(__git_ps1 " (%s)")]\$ ' fi else PS1='[\u\@\h \W$(__git_ps1 " (%s)")]\$ ' fi if [ -f "/usr/share/git-core/contrib/completion/git-prompt.sh" ]; then source /usr/share/git-core/contrib/completion/git-prompt.sh fi This makes your shell navigating in to a GIT repository appear as follows: #. :command:`cd` in to a GIT repository: .. parsed-literal:: [kanarip\@dws06 ~]$ :command:`cd ~/devel/src/kolab/pykolab.git` [kanarip\@dws06 pykolab.git (master u=)]$ This means a clean working copy. #. Create an untracked file: .. parsed-literal:: [kanarip\@dws06 pykolab.git (master u=)]$ :command:`touch something` [kanarip\@dws06 pykolab.git (master % u=)]$ THe `%` means untracked files exist in the directory hierarchy. #. Add the untracked file: .. parsed-literal:: [kanarip\@dws06 pykolab.git (master % u=)]$ :command:`git add something` [kanarip\@dws06 pykolab.git (master + u=)]$ The `+` means tracked, uncommitted files exist in the directory hierarchy. #. Change a file: .. parsed-literal:: [kanarip\@dws06 pykolab.git (master + u=)]$ :command:`echo 1 > something` [kanarip\@dws06 pykolab.git (master \*+ u=)]$ The `*` means uncommitted changes to tracked files exist. The `+` still indicates a tracked file is not yet committed. #. Checkout another branch. In this example, it is specifically made dirty to show off: .. parsed-literal:: [kanarip\@dws06 pykolab.git (master \*+ u=)]$ :command:`git checkout pykolab-0.7` A something Switched to branch 'pykolab-0.7' Your branch and 'origin/pykolab-0.7' have diverged, and have 4 and 65 different commits each, respectively. (use "git pull" to merge the remote branch into yours) [kanarip\@dws06 pykolab.git (pykolab-0.7 \*+ u+4-65)]$ This means we have 4 commits to our local working copy not yet in the remote tracked, and 65 commits in the remote tracked not yet in our local working copy. #. Attempt to rebase on top of the tracked remote: .. parsed-literal:: [kanarip\@dws06 pykolab.git (pykolab-0.7 \*+ u+4-65)]$ :command:`git rebase origin/pykolab-0.7 --autostash` Created autostash: 49f31f4 HEAD is now at a6fb106 Ensure docker runs on atomic hosts First, rewinding head to replay your work on top of it... Applied autostash. [kanarip\@dws06 pykolab.git (pykolab-0.7 + u=)]$ You'll notice the `+` again stands for the tracked, not yet committed file :file:`something`. -Creating a Differential for Review -================================== - -Your development takes place against the **master** branch, unless you find -yourself running in circles, interrupted by a support request. - -Thus, make sure you have **master** checked out, and for the sake of preventing -superfluous merge and rebase exercises, ensure it's in sync with upstream: - -.. parsed-literal:: - - $ :command:`git checkout master` - $ :command:`git fetch origin` - $ :command:`git rebase origin/master --autostash` - -Provided a ticket, such as :task:`1037`, you can branch off the GIT repository; - -.. parsed-literal:: - - [kanarip\@dws06 docs.git (master u=)]$ :command:`git checkout -b dev/T1037` - [kanarip\@dws06 docs.git (dev/T1037)]$ - -Make your changes, and commit them in however many commits you think is -reasonable. - -Then, create the `Differential`_: - -.. parsed-literal:: - - [kanarip\@dws06 docs.git (dev/T1037 %)]$ arc diff - You have untracked files in this working copy. - - Working copy: /home/kanarip/devel/src/kolab/docs.git/ - - Untracked changes in working copy: - (To ignore these changes, add them to ".git/info/exclude".) - quick-notes.txt - - Ignore these untracked files and continue? [y/N] :command:`y` - -You will now be requested to provide some information about your proposed -changes. - -Set the first ``Summary:`` line to ``Resolves T1037``, so that your -differential will be associated with the ticket automatically, and an accepted -differential also closes the ticket you refer to. - -.. parsed-literal:: - - Linting... - No lint engine configured for this project. - Running unit tests... - No unit test engine is configured for this project. - Updating commit message... - Created a new Differential revision: - Revision URI: https://git.kolab.org/D89 - - Included changes: - M source/index.rst - A source/contributor-guide/index.rst - -And that's it.