Skip to content
Snippets Groups Projects
Commit 29ada725 authored by David Douard's avatar David Douard
Browse files

Remove old contributing/phabricator page

parent f1d69cd6
No related branches found
No related tags found
1 merge request!468Several minor updates/improvements and remove the phabricator page
Hide whitespace changes
Inline Side-by-side
docs/devel/contributing/phabricator.rst deleted 100644 → 0
+ 0
− 308
View file @ f1d69cd6
:orphan:
.. highlight:: bash
.. admonition:: Intended audience
:class: important
Contributors
Important
=========
We have moved our development from Phabricator to a GitLab instance at
https://gitlab.softwareheritage.org/
The content below is no longer relevant and will be updated soon.
Submitting patches
==================
`Phabricator`_ is the tool that Software Heritage uses as its
coding/collaboration forge.
Software Heritage's Phabricator instance can be found at
https://forge.softwareheritage.org/
.. _Phabricator: http://phabricator.org/
Code Review in Phabricator
--------------------------
We use the Differential application of Phabricator to perform
:ref:`code reviews <code-review>` in the context of Software Heritage.
* we use Git and ``history.immutable=true``
(but beware as that is partly a Phabricator misnomer, read on)
* when code reviews are required, developers will be allowed to push
directly to master once an accepted Differential diff exists
Configuration
+++++++++++++
.. _arcanist-configuration:
Arcanist configuration
^^^^^^^^^^^^^^^^^^^^^^
Authentication
~~~~~~~~~~~~~~
First, you should install Arcanist and authenticate it to Phabricator::
sudo apt-get install arcanist
arc set-config default https://forge.softwareheritage.org/
arc install-certificate
arc will prompt you to login into Phabricator via web
(which will ask your personal Phabricator credentials).
You will then have to copy paste the API token from the web page to arc,
and hit Enter to complete the certificate installation.
Immutability
~~~~~~~~~~~~
When using git, Arcanist by default mess with the local history,
rewriting commits at the time of first submission.
To avoid that we use so called `history immutability`_
.. _history immutability: https://secure.phabricator.com/book/phabricator/article/arcanist_new_project/#history-mutability-git
To that end, you shall configure your ``arc`` accordingly::
arc set-config history.immutable true
Note that this does **not** mean that you are forbidden to rewrite
your local branches (e.g., with ``git rebase``).
Quite the contrary: you are encouraged to locally rewrite branches
before pushing to ensure that commits are logically separated
and your commit history easy to bisect.
The above setting just means that *arc* will not rewrite commit
history under your nose.
Enabling ``git push`` to our forge
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The way we've configured our review setup for continuous integration
needs you to configure git to allow pushes to our forge.
There's two ways you can do this : setting a ssh key to push over ssh,
or setting a specific password for git pushes over https.
SSH key for pushes
~~~~~~~~~~~~~~~~~~
In your forge User settings page (On the top right, click on your avatar,
then click *Settings*), you have access to a *Authentication* >
*SSH Public Keys* section (Direct link:
``hxxps://forge.softwareheritage.org/settings/user/<your username>/page/ssh/``).
You then have the option to upload a SSH public key,
which will authenticate your pushes.
You then need to configure ssh/git to use that key pair,
for instance by editing the ``~/.ssh/config`` file.
Finally, you should configure git to push over ssh when pushing to
https://forge.softwareheritage.org, by running the following command::
git config --global url.git@forge.softwareheritage.org:.pushInsteadOf https://forge.softwareheritage.org
This lets git know that it should use ``git@forge.softwareheritage.org:``
as a base url when pushing repositories cloned from
forge.softwareheritage.org over https.
VCS password for pushes
~~~~~~~~~~~~~~~~~~~~~~~
.. warning:: Please, only use this if you're completely unable to use ssh.
As a fallback to the ssh setup, you have the option of setting a VCS password. This
password, *separate from your account password*, allows Phabricator to authenticate your
uploads over HTTPS.
In your forge User settings page (On the top right, click on your avatar, then click
*Settings*), you need to use the *Authentication* > *VCS Password* section to set your
VCS password (Direct link: ``hxxps://forge.softwareheritage.org/settings/user/<your
username>/page/vcspassword/``).
If you still get a 403 error on push, this means you need a forge administrator to
enable HTTPS pushes for the repository (which wasn't done by default in historical
repositories). Please drop by on IRC and let us know!
Workflow
++++++++
* work in a feature branch: ``git checkout -b my-feat``
* initial review request: hack/commit/hack/commit ;
``arc diff origin/master``
* react to change requests: hack/commit/hack/commit ;
``arc diff --update Dxx origin/master``
* landing change: ``git checkout master ; git merge my-feat ; git push``
Starting a new feature and submit it for review
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Use a **one branch per feature** workflow, with well-separated
**logical commits** (:ref:`following those conventions <git-style-guide>`).
Please open one diff per logical commit to keep the diff size to a minimum.
.. code-block::
git checkout -b my-shiny-feature
... hack hack hack ...
git commit -m 'architecture skeleton for my-shiny-feature'
... hack hack hack ...
git commit -m 'my-shiny-feature: implement module foo'
... etc ...
Please, follow the
To **submit your code for review** the first time::
arc diff origin/master
arc will prompt for a **code review message**. Provide the following information:
* first line: *short description* of the overall work
(i.e., the feature you're working on).
This will become the title of the review
* *Summary* field (optional): *long description* of the overall work;
the field can continue in subsequent lines, up to the next field.
This will become the "Summary" section of the review
* *Test Plan* field (optional): write here if something special is needed
to test your change
* *Reviewers* field (optional): the (Phabricator) name(s) of
desired reviewers.
If you don't specify one (recommended) the default reviewers will be chosen
* *Subscribers* field (optional): the (Phabricator) name(s) of people that
will be notified about changes to this review request.
In most cases it should be left empty
For example::
mercurial loader
Summary: first stab at a mercurial loader (T329)
The implementation follows the plan detailed in F2F discussion with @foo.
Performances seem decent enough for a first trial (XXX seconds for YYY repository
that contains ZZZ patches).
Test plan:
Reviewers:
Subscribers: foo
After completing the message arc will submit the review request
and tell you its number and URL::
[...]
Created a new Differential revision:
Revision URI: https://forge.softwareheritage.org/Dxx
.. _arc-update:
Updating your branch to reflect requested changes
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Your feature might get accepted as is, YAY!
Or, reviewers might request changes; no big deal!
Use the Differential web UI to follow-up to received comments, if needed.
To implement requested changes in the code, hack on your branch as usual by:
* adding new commits, and/or
* rewriting old commits with git rebase (to preserve a nice, easy to bisect history)
* pulling on master and rebasing your branch against it if meanwhile someone
landed commits on master:
.. code-block::
git checkout master
git pull
git checkout my-shiny-feature
git rebase master
When you're ready to **update your review request**::
arc diff --update Dxx HEAD~
Arc will prompt you for a message: **describe what you've changed
w.r.t. the previous review request**, free form.
This means you should not repeat the title of your diff (which is
often the default if you squashed/amended your commits)
Your message will become the changelog entry in Differential
for this new version of the diff, and will help reviewers
understand what changes you made since they last read your diff.
Differential only care about the code diff, and not about the commits
or their order.
Therefore each "update" can be a completely different series of commits,
possibly rewritten from the previous submission.
Dependencies between diffs
^^^^^^^^^^^^^^^^^^^^^^^^^^
Note that you can manage diff dependencies within the same module
with the following keyword in the diff description::
Depends on Dxx
That allows to keep a logical view in your diff.
It's not strictly necessary (because the tooling now deals with it properly)
but it might help reviewers or yourself to do so.
Landing your change onto master
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Once your change has been approved in Differential,
you will be able to land it onto the master branch.
Before doing so, you're encouraged to **clean up your git commit history**,
reordering/splitting/merging commits as needed to have separate
logical commits and an easy to bisect history.
Update the diff :ref:`following the prior section <arc-update>`
(It'd be good to let the CI build finish to make sure everything is still green).
Once you're happy you can **push to origin/master** directly, e.g.::
git checkout master
git merge --ff-only my-shiny-feature
git push
``--ff-only`` is optional, and makes sure you don't unintentionally
create a merge commit.
Optionally you can then delete your local feature branch::
git branch -d my-shiny-feature
Reviewing locally / landing someone else's changes
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
You can do local reviews of code with arc patch::
arc patch Dxyz
This will create a branch **arcpatch-Dxyz** containing the changes
on your local checkout.
You can then merge those changes upstream with::
git checkout master
git merge --ff arcpatch-Dxyz
git push origin master
or, alternatively::
arc land --squash
See also
--------
* :ref:`code-review` for guidelines on how code is reviewed
when developing for Software Heritage
docs/sysadm/puppet/howto-manage-third-party-modules.rst
+ 3
− 10
View file @ 29ada725
......@@ -29,12 +29,6 @@ specifier.
Adding a new external puppet module
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. admonition:: Deprecation notice
:class: important
This needs refactoring as the script mentioned in this section is not yet ported to
use our new gitlab instance
In the *puppet-environment* repository, the ``bin/import-puppet-module`` takes care of
the following tasks:
......@@ -49,10 +43,9 @@ the following tasks:
To be able to use the script, you need:
- Be a member of the `System Administrators
<https://forge.softwareheritage.org/project/members/7/>`_ Phabricator group
- Have the :ref:`Arcanist <arcanist-configuration>` API key setup
- A pair of python dependencies: ``python3-phabricator`` and ``python3-requests`` (pull
them from testing if needed).
<https://gitlab.softwareheritage.org/groups/teams/sysadmin>`_ gitlab group
- Have the gitlab API key setup
- A few python dependencies: ``python3-click``, ``python3-gitlab`` and ``python3-requests``.
Example usage to pull the `elastic/elasticsearch
<https://forge.puppetlabs.com/elastic/elasticsearch>`_ module
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment