launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28883
[Merge] ~cjwatson/launchpad:style-guide-black into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:style-guide-black into launchpad:master.
Commit message:
Update style guide to use black
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/427354
Also update a few bits of example code in documentation to use `black`-style formatting.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:style-guide-black into launchpad:master.
diff --git a/doc/how-to/preserve-query-count.rst b/doc/how-to/preserve-query-count.rst
index 2b8e180..aa88c37 100644
--- a/doc/how-to/preserve-query-count.rst
+++ b/doc/how-to/preserve-query-count.rst
@@ -15,10 +15,13 @@ as outlined in the following example:
person = self.factory.makePerson()
pillarperson = PillarPerson(self.pillar, person)
recorder1, recorder2 = record_two_runs(
- tested_method=lambda: create_initialized_view(pillarperson, '+index'),
- item_creator=lambda: self.makeArtifactGrantee(person, True, True, True, False),
+ tested_method=lambda: create_initialized_view(pillarperson, "+index"),
+ item_creator=lambda: self.makeArtifactGrantee(
+ person, True, True, True, False
+ ),
first_round_number=5,
- login_method=lambda: login_person(self.owner))
+ login_method=lambda: login_person(self.owner),
+ )
self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
For further information, please have a look at `record_two_runs`' docstring.
diff --git a/doc/how-to/running.rst b/doc/how-to/running.rst
index affb999..574f9b6 100644
--- a/doc/how-to/running.rst
+++ b/doc/how-to/running.rst
@@ -208,11 +208,13 @@ You are now in a newly-cloned Git repository, with one branch ('master'),
into whose working tree the other source dependencies have been symlinked.
The source dependencies actually live in ``../lp-sourcedeps``.
+.. _pre-commit:
+
Installing the pre-commit hook
==============================
If you intend to make any changes to Launchpad, you should also set up
-`pre-commit <https://pre-commit.com/>`_ now:
+`pre-commit <https://pre-commit.com/>`__ now:
1. Install ``pre-commit`` itself. If your host system is Ubuntu 20.10 or
newer, then ``sudo apt install pre-commit`` is enough; otherwise, you can
diff --git a/doc/how-to/security.rst b/doc/how-to/security.rst
index 0435a8a..bc18080 100644
--- a/doc/how-to/security.rst
+++ b/doc/how-to/security.rst
@@ -49,7 +49,7 @@ To define a security adapter for a given permission on an interface:
.. code-block:: python
class EditByOwner(AuthorizationBase):
- permission = 'launchpad.Edit'
+ permission = "launchpad.Edit"
usedfor = IHasOwner
def checkAuthenticated(self, user):
diff --git a/doc/reference/python.rst b/doc/reference/python.rst
index 2725630..e3a76ab 100644
--- a/doc/reference/python.rst
+++ b/doc/reference/python.rst
@@ -36,40 +36,15 @@ Related Documents
* `Launchpad hacking FAQ <https://dev.launchpad.net/LaunchpadHackingFAQ>`_
* :doc:`Tests style guide <tests>`
-Whitespace and Wrapping
-=======================
-
-(Most of this is likely to be delegated to `black
-<https://github.com/psf/black>`_ in the near future.)
-
-* Code should fit within 78 columns, so as to fit nicely in an 80 column
- terminal, even when quoted in an email.
-* Indents should be 4 spaces.
-* **No tabs**. This is not negotiable.
-
-.. _multiline:
-
-Multiline braces
-----------------
-
-There are lots of cases where you might have a list, tuple or dictionary
-literal that spans multiple lines. In these cases, you should consider
-formatting these literals as follows. This format makes changes to the list
-clearer to read in a diff. Note the trailing comma on the last element.
-
-.. code-block:: python
-
- mydict = {
- 'first': 1,
- 'second': 2,
- 'third': 3,
- }
+Formatting
+==========
- mylist = [
- 'this is the first line',
- 'this is the second line',
- 'this is the third line',
- ]
+We delegate most formatting decisions to `black
+<https://github.com/psf/black>`_. All Python code (except for a few files
+specifically excluded in ``.pre-commit-config.yaml``) must be formatted
+using it. You should be using Launchpad's default :ref:`pre-commit
+<pre-commit>` setup, which automatically formats your code using ``black``
+and ``isort`` before you commit.
Naming
======
@@ -148,7 +123,7 @@ Each module should look like this:
__all__ = [
...
- ]
+ ]
The file ``standard_template.py`` has most of this already, so save yourself
time by copying that when starting a new module. The "..." should be filled
@@ -183,46 +158,6 @@ tests not to pass if you don't abide by the rules.
Use absolute imports (``from foo.bar import Bar``), not relative imports
(``from .bar import Bar``).
-Multiline imports
------------------
-
-You should be using Launchpad's default `pre-commit
-<https://dev.launchpad.net/Running#pre-commit>`_ setup, which automatically
-formats your imports using ``isort`` before you commit. The remainder of
-this section is for information.
-
-Sometimes import lines must span multiple lines, either because the package
-path is very long or because there are multiple names inside the module that
-you want to import.
-
-**Never use backslashes in import statements!** Use parenthesized imports:
-
-.. code-block:: python
-
- from foo import (
- That,
- TheOther,
- This,
- )
-
-Like other lists, imports should list one item per line. The exception is
-if only one symbol is being imported from a given module.
-
-.. code-block:: python
-
- from lp.app.widgets.itemswidgets import CheckBoxMatrixWidget
-
-But if you import two or more, then each item needs to be on a line by
-itself. Note the trailing comma on the last import and that the closing
-paren is on a line by itself.
-
-.. code-block:: python
-
- from lp.app.widgets.itemswidgets import (
- CheckBoxMatrixWidget,
- LaunchpadRadioWidget,
- )
-
Import scope
------------
@@ -280,11 +215,11 @@ For example:
from lp.services.webservice.apihelpers import (
patch_entry_return_type,
patch_collection_return_type,
- )
+ )
patch_collection_return_type(
- IArchive, 'getComponentsForQueueAdmin', IArchivePermission)
- patch_entry_return_type(
- IArchive, 'newPackageUploader', IArchivePermission)
+ IArchive, "getComponentsForQueueAdmin", IArchivePermission
+ )
+ patch_entry_return_type(IArchive, "newPackageUploader", IArchivePermission)
Properties
==========
@@ -363,7 +298,7 @@ queries or fragments, e.g.:
FROM TeamParticipation
INNER JOIN Person ON TeamParticipation.team = Person.id
WHERE TeamParticipation.person = %s
- """ % sqlvalues(personID)
+ """ % sqlvalues(personID)
This is also easy to cut-and-paste into ``psql`` for interactive testing,
unlike if you use several lines of single quoted strings.
@@ -395,9 +330,9 @@ Or:
.. code-block:: python
fd, filename = mkstemp()
- with os.fdopen(fd, 'w') as temp_file:
+ with os.fdopen(fd, "w") as temp_file:
...
- temp_file.write('foo')
+ temp_file.write("foo")
**Never** use:
@@ -405,7 +340,7 @@ Or:
fd, filename = mkstemp()
with open(filename) as temp_file:
- temp_file.write('foo')
+ temp_file.write("foo")
# BOOM! 'fd' leaked.
In tests, you should use the ``TempDir`` fixture instead, which cleans
@@ -420,7 +355,7 @@ itself up automatically:
def test_foo(self):
tempdir = self.useFixture(TempDir).path
...
- do_something(os.path.join(tempdir, 'test.log'))
+ do_something(os.path.join(tempdir, "test.log"))
...
Configuration hints
diff --git a/doc/reference/tests.rst b/doc/reference/tests.rst
index 34a3267..a597641 100644
--- a/doc/reference/tests.rst
+++ b/doc/reference/tests.rst
@@ -125,14 +125,14 @@ The basic conventions for testable documentation are:
You use the getFoo() method to obtain an IFoo instance by id:
- >>> foo = foobarset.getFoo('aFoo')
+ >>> foo = foobarset.getFoo("aFoo")
>>> verifyObject(IFoo, foo)
True
Similarly, you use the getBar() method to retrieve an IBar instance by
id:
- >>> bar = foobarset.getBar('aBar')
+ >>> bar = foobarset.getBar("aBar")
>>> verifyObject(IBar, bar)
True
@@ -160,7 +160,7 @@ will be helpful to debug the problem. Avoid constructs like:
.. code-block:: pycon
- >>> 'Test' in foo.getText()
+ >>> "Test" in foo.getText()
True
The failure message for this test will be:
@@ -188,16 +188,17 @@ easy to access specific elements in the tree.
The new status is displayed in the portlet.
- >>> details_portlet = find_portlet(browser.contents, 'Question details')
- >>> print(details_portlet.find('b', text='Status:').next.strip())
+ >>> details_portlet = find_portlet(browser.contents, "Question details")
+ >>> print(details_portlet.find("b", text="Status:").next.strip())
Needs information
There is also an ``extract_text()`` helper that only renders the HTML text:
.. code-block:: pycon
- >>> print(extract_text(
- ... find_tag_by_id(browser.contents, 'branchtable')))
+ >>> print(
+ ... extract_text(find_tag_by_id(browser.contents, "branchtable"))
+ ... )
main 60 New firefox
klingon 30 Experimental gnome-terminal
junk.contrib 60 New 2005-10-31 12:03:57 ... weeks ago
@@ -451,13 +452,13 @@ human-readable. So:
.. code-block:: python
- self.assertTrue('aString' in result)
+ self.assertTrue("aString" in result)
could be replaced by:
.. code-block:: python
- self.assertIn('aString', result)
+ self.assertIn("aString", result)
* Consider using testtools matchers where reasonable. These can often
improve failure messages so that they show more information in one go,
@@ -466,16 +467,16 @@ could be replaced by:
.. code-block:: python
- self.assertEqual('expected', obj.foo)
- self.assertEqual('more expected', obj.bar)
+ self.assertEqual("expected", obj.foo)
+ self.assertEqual("more expected", obj.bar)
prefer this:
.. code-block:: python
self.assertThat(obj, MatchesStructure.byEquality(
- foo='expected',
- bar='more expected'))
+ foo="expected",
+ bar="more expected"))
In general, you should follow Launchpad coding conventions (see :doc:`Python
style guide <python>`), however when naming test methods:
@@ -500,7 +501,7 @@ is available as a class method on ``BaseLayer``.
.. code-block:: python
- def appserver_root_url(self, facet='mainsite', ensureSlash=False):
+ def appserver_root_url(self, facet="mainsite", ensureSlash=False):
"""Return the correct app server root url for the given facet."""
Code snippets for a number of scenarios are as follows.
@@ -522,4 +523,4 @@ Code snippets for a number of scenarios are as follows.
def test_replay_attacks_do_not_succeed(self):
browser = Browser(mech_browser=MyMechanizeBrowser())
- browser.open('%s/+login' % self.layer.appserver_root_url())
+ browser.open("%s/+login" % self.layer.appserver_root_url())
