Remove docformatter as a pre-commit hook

[rmartin16]

Small fixes to make these formatters happy(er)...

Relevant issue: PyCQA/docformatter#154

Removed docformatter as a pre-commit hook. By and large, black serves the purpose of formatting docstrings.

Because removing docformatter, I ran it one last time using black's default docstring width of 88 characters.

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[freakboy3742]

So - these changes all look fine... at the very least, the missing tests are a big review OOPS on my part - but I'm not sure I'm following what the actual problem is here. You seem to be suggesting there's a code-style issue... but I haven't seen any problems reported by pre-commit, and there's no pre-commit configuration change associated with this PR that might induce one.

Can you elaborate on how you're observing the problem that this PR fixes?

[freakboy3742]

I was clearly having a good day when I approved this tests the first time around...

[rmartin16]

These are problematic in versions of linters we aren't currently using. The next release of docformatter, v1.6.0, is currently not handling "empty" functions very well. It is documented in PyCQA/docformatter#154.

[rmartin16]

As an example, here's what docformatter==1.6.0-rc2 was doing to linuxdeploy.py:

❯ docformatter src/briefcase/integrations/linuxdeploy.py 
--- before/src/briefcase/integrations/linuxdeploy.py
+++ after/src/briefcase/integrations/linuxdeploy.py
@@ -35,12 +35,10 @@
     @abstractmethod
     def download_url(self):
         """The URL where the tool/plugin can be downloaded."""
-
     @property
     @abstractmethod
     def file_path(self):
         """The folder on the local filesystem that contains the file_name."""
-
     def exists(self):
         return (self.file_path / self.file_name).exists()

Also, I wouldn't care so much about these RC releases except pre-commit autoupdate will use them to update .pre-commit-config.yaml.

[freakboy3742]

Ok - that makes sense. However, I wonder if the fix is to remove the docformatter pre-commit.

We haven't enabled it on Toga and Rubicon because it chokes hard on formatting of ReST references (PyCQA/docformatter#124). At the end of the day, I don't know I've seen docformatter pick up any problems that I'd actually consider bugs, outside of line length issues that are already picked up by black - but it definitely likes reformatting docstrings in a way that hits the pre-commit check on almost every test I've ever written.

The 2 missing tests definitely need to be preserved, though :-)

[rmartin16]

The primary role that docformatter seems to be serving for Briefcase is limiting the first line of a docstring to 79 characters and subsequent lines to 72 characters. Alternatively, black seems to limit both of these to 88 characters.

@freakboy3742, before I get rid of docformatter, do you think it would be worth me running it with a 88 character limit to align all the docstrings better with black?

[freakboy3742]

I'm happy to live with an 88 character limit; if it's easy to reprocess the docstrings to get them out to an 88 char limit, then sure, but if it's more than a couple of minutes work, I don't think it matters that much if existing lines stay 9-16 chars short.

[rmartin16]

It's simple; just add --wrap-descriptions=88, --wrap-summaries=88 to the args for docformatter in .pre-commit-config.yaml.

[freakboy3742]

A bit of a monster diff, but definitely worth it. The number of "barely over 79/72 char" lines that are cleaned up speaks for itself, IMHO.

[rmartin16]

FYI; I realized today that docformatter was also the one responsible for ensuring the first line of all of our docstrings end in a period. Not huge....but I'll probably miss that a little bit.

[freakboy3742]

Agreed it's annoying, but not the end of the world. We'll just have to remember how to punctuate