From e08f0ed0439e994b0f4415159270605973d9b084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Tue, 24 Jan 2023 21:52:38 +0100 Subject: [PATCH] Revamp the contributor guide While some sections feel redundant, other sections were missing, like the one about choosing the right branch. --- source/contribute.rst | 296 +++++++++++++++++++++--------------------- 1 file changed, 148 insertions(+), 148 deletions(-) diff --git a/source/contribute.rst b/source/contribute.rst index 83b9bb52..80a5249b 100644 --- a/source/contribute.rst +++ b/source/contribute.rst @@ -65,112 +65,140 @@ Here is how you can make your local repository aware of your fork: You should now have two remotes: one called ``doctrine``, another called ``fork``. -Branching from the default branch ---------------------------------- +Choosing the right branch +------------------------- + +Before you start working on a feature or a bug fix, you need to choose +which branch you are going to contribute to. We try hard to follow +semver, which means there are three choices. + +Let's say the latest version of the project is 2.3.1. The following +branches should exist and are the possible choices: + +- ``2.3.x`` from which the next patch release will be tagged (2.3.2, 2.3.3, etc.); +- ``2.4.x`` from which the next minor release will be tagged (2.4.0); +- ``3.0.x`` from which the next major release will be tagged (3.0.0). + +To find out what's the latest version, you can check the "Releases" +block on the project's Github page. Alternatively, you can check +Packagist. + +Anything you contribute to a branch will be merged up to higher branches +at some point. For example, if you contribute to ``2.3.x``, your changes +will eventually land on ``2.4.x`` and ``3.0.x``. + + +The general rules are as follows: + +- Patch releases should be as stable as possible. They fix bugs, but in + avoid making changes as far as possible. +- Improvements and additions are being made on the upcoming minor + release branch. +- An upgrade to a new minor version must not require any changes on the + user's side. +- There must be a clear upgrade path from one major version to the next. + That means necessary changes on the user's side will be advertised + through deprecation notices, if anyway possible. +- Deprecations may be added for new minor versions only. +- Before a feature is removed (or an API changed) in a major version + release, a corresponding deprecation notice must be included in a + minor version release preceding it. +- If a replacement feature and/or alternative API will be provided, it + must be available in the minor release that adds the deprecation for + the old feature. + +With that in mind: + +If you are touching something outside the source directory (usually +``src``, sometimes ``lib``), and that thing is not ``composer.json``, +you should probably target the lowest branch. +There is no way that your contribution is going to create a bug for +the end user, so let us get it on all 3 branches with merges up. +Examples are: documentation, tests, build scripts, tooling configuration. + +If you are touching something inside the source directory, you may +still contribute to the lowest branch if it is a bug fix, and if you +are touching comments that are not phpdoc comments. + +If the comments are phpdoc comments… it depends. If the phpdoc was +inaccurate, you should target the lowest branch. If the phpdoc was +imprecise but technically correct, you should target the next minor +branch. + +If you are doing a refactoring and you think you didn't change the +behavior of the code, you should still target the next minor branch +unless the refactoring is necessary for a bugfix. Let us not take any +unnecessary risks. + +If you want to deprecate something, you should target the next minor +branch: upgrading to a new patch version should not result in any +additional deprecations. + +If you want to add a new feature, you should target the next minor +branch, because it might have unintentional side effects that break existing code. + +If you want to remove or change a public API, you should target the next +major branch. If you change a public API, you should provide an upgrade +path for the end user. Ideally, you should make the new API available on +the next minor branch, deprecate the old API, and remove it in the next +major. + +Have you made your choice? Good. You now need to create a topic branch. -New pull requests are created with the repository's default branch as base branch. -The default branch is the branch you see when you enter the repository page on GitHub. - -.. image:: ../images/default-branch.png - :alt: The default branch - :style: margin-bottom: 20px - -In this DBAL example, it's the branch with the name **2.11.x**. The branch name reflects the -current lowest supported version of a repository. - -Newly introduced changes to 2.11.x will be up-merged at a later point in time -to newer version branches (e.g. 2.12.x, 3.0.x). This way you don't have to -re-introduce a new fix or feature of 2.11.x with another pull request to -the other version branches. - -Keeping the default branch up-to-date! --------------------------------------- +Creating a topic branch +----------------------- -Once all this is done, you'll be able to keep your local branches up to -date with the following command: +Time might have elapsed since the last time you contributed or since you +cloned the repository. You might want to fetch the latest changes from +the ``doctrine`` remote: .. code-block:: console $ git fetch doctrine -Branching Model ---------------- - -The following names will be used to differentiate between the different -repositories: - -- **doctrine** - The "official" Doctrine DBAL repository -- **fork** - Your fork of the official repository on GitHub -- **local** - This will be your local clone of **fork** - -As a **contributor** you will push your completed **local** topic branch -to **fork**. As a **contributor** you will pull updates from -**doctrine**. As a **maintainer** (write-access) you will merge branches -from contributors into **doctrine**. +I know it sounds awful, but the next step is to deal with one of the 2 +hard problems in computer science and come up with a name for your +branch. Pick something meaningful. -Primary Branches ----------------- +If you have a feature to contribute that adds support for a new database +called YourSQL, you could create a branch called ``your-sql-support`` +from the next minor branch: -The **doctrine** repository holds the following primary branches: - -- **doctrine/2.11.x** Development towards the next release. -- **doctrine/\*** Maintenance branches of existing releases. - -These branches exist in parallel and are defined as follows: - -**doctrine/2.11.x** is the branch where the source code of **HEAD** -always reflects the latest version. Each released stable version will be -a tagged commit in a **doctrine/\*** branch. Each released unstable -version will be a tagged commit in the **doctrine/2.11.x** branch. +.. code-block:: console - **NOTE** You should never commit to your forked default branch (**fork/2.11.x**). - Changes to **fork/2.11.x** will never be merged into - **doctrine/2.11.x**. All work must be done in a **topic branch**, - which are explained below. + $ git switch --create your-sql-support doctrine/2.4.x # use -c for short -Topic Branches --------------- +Here, using ``doctrine/2.4.x`` instead of just ``2.4.x`` means you do +not have to switch to 2.4.x and update it first. In fact, unless you are +a maintainer, you are never supposed to commit on that branch. One way +to avoid that mistake is to delete that branch: -Topic branches are for contributors to develop bug fixes, new features, -etc. so that they can be easily merged to **2.11.x**. They must follow a -few rules as listed below: +.. code-block:: console -- May branch off from: **2.11.x** whenever possible, or a newer version - branch otherwise. Keep in mind that your changes will be - up-merged to higher version branches by maintainers after the merge if - they are applicable. -- Branch naming convention: anything except master, the default branch name, - or version branch names. + $ git branch --delete 2.4.x # use -d for short -Topic branches are used to develop new features and fix reported issues. -When starting development of a feature, the target release in which this -feature will be incorporated may well be unknown. The essence of a topic -branch is that it exists as long as the feature is in development, but -will eventually be merged into **2.11.x** or a release branch (to -add the new feature or bugfix to a next release) or discarded (in case -of a disappointing experiment). +Later, if you want to see how things are on ``doctrine/2.4.x``, you can +use ``--detach``: -Topic branches should exist in your **local** and **fork** -repositories only, there is no need for them to exist in **doctrine**. +.. code-block:: console -Creating a topic branch ------------------------ + $ git switch --detach doctrine/2.4.x # use -d for short -First create an appropriately named branch. When starting work on a new -topic, branch off from **doctrine/2.11.x** or a **doctrine/\*** branch: +Now do your changes, and when you are done, you need to commit them. +To pick the right changes, we recommend you use ``git add --patch``. It +will force you to review what you are about to commit. .. code-block:: console - $ git checkout -b fix-weird-bug doctrine/2.11.x - Switched to a new branch "fix-weird-bug" + $ git add --patch # use -p for short -Now do some work, make some changes then commit them: +Likewise, when you commit, we recommend you use ``--verbose``. It will +show the diff again in your editor. -.. code-block:: console + $ git commit --verbose # use -v for short - $ git status - $ git add -p - $ git commit -v +It is important that you pause here and make the effort of writing a +meaningful commit message. Crafting meaningful commit messages ----------------------------------- @@ -206,6 +234,31 @@ this in case you want to read more about this: - `Commit message style for git `_ - `A note about git commit messages `_ +Once you are done, you can push your branch to your fork: + +.. code-block:: console + + $ git push --set-upstream fork your-sql-support # use -u for short + +Creating the pull request +------------------------- + +New pull requests are created with the repository's default branch as +base branch, and that might not be what you want. Make sure to pick the +correct branch when creating the pull request. If you do not, it can +still be changed afterwards. + +The default branch is the branch you see when you enter the repository +page on GitHub. + +.. image:: ../images/default-branch.png + :alt: The default branch + :style: margin-bottom: 20px + +In this DBAL example, it's the branch with the name **2.11.x**. The +branch name reflects the current lowest supported version of a +repository. + To squash or not to squash -------------------------- @@ -278,73 +331,20 @@ with both the code and the docs here. Rebasing on upstream changes ---------------------------- -Next, merge or rebase your commit against **doctrine/2.11.x**. With your -work done in a **local** topic branch, you'll want to assist upstream -merge by rebasing your commits. You can either do this manually with -``fetch`` then ``rebase``, or use the ``pull --rebase`` shortcut. You -may encounter merge conflicts, which you should fix and then mark as -fixed with ``add``, and then continue rebasing with -``rebase --continue``. At any stage, you can abort the rebase with -``rebase --abort`` unlike nasty merges which will leave files strewn -everywhere. - -.. code-block:: console - - $ git fetch doctrine - $ git rebase doctrine/2.11.x fix-weird-bug - -Push your branch to **fork**: - -Finished topic branches should be pushed to **fork** for a -**maintainer** to review and pull into **doctrine** as appropriate: - -.. code-block:: console - - $ git push fork fix-weird-bug - To git@github.com:hobodave/dbal.git - * [new branch] fix-weird-bug -> fix-weird-bug - -Now you are ready to send a pull request from this branch and ask for a -review from a maintainer. - -Topic Branch Cleanup --------------------- - -Once your work has been merged by the branch maintainer, it will no -longer be necessary to keep the local branch or remote branch, so you -can remove them! +Sometimes, you will need to rebase your branch on the latest changes, +typically because the build had an issue unrelated to your changes, and +that issue has been fixed after you created your branch. -Sync your local 2.11.x branch: - -.. code-block:: console - - $ git checkout 2.11.x - $ git pull --rebase - -Remove your local topic branch using -d to ensure that it has been merged by -upstream. Branch -d will not delete a branch that is not an ancestor of -your current head. - -From the git-branch man page: - -.. code-block:: console - - -d - Delete a branch. The branch must be fully merged in HEAD. - -D - Delete a branch irrespective of its merged status. - -Remove your local topic branch: - -.. code-block:: console - - $ git branch -d fix-weird-bug - -Remove your remote branch at **fork**: - -.. code-block:: console +Here is how to proceed if you need to rebase on ``2.3.x``: - $ git push fork :fix-weird-bug +1. Fetch all new commits: `git fetch doctrine``. +2. Rebase on what you fetched interactively: + ``git rebase --interactive doctrine/2.3.x # use -i for short`` +3. A window will show up with many lines, remove every line but the last + ones, which should correspond to your commits. +4. If you run into a conflict, fix it with ``git mergetool``, then + continue on your merry way with ``git rebase --continue``. +5. Force push to overwrite the previous version : ``git push --force``. Project Dependencies --------------------