Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Would you consider using code formatters? #191

Closed
kurtmckee opened this issue Sep 26, 2024 · 4 comments
Closed

Would you consider using code formatters? #191

kurtmckee opened this issue Sep 26, 2024 · 4 comments

Comments

@kurtmckee
Copy link
Contributor

@LudovicRousseau I'd like to use the following code formatters on the codebase, but want to get your feedback first:

  • pyupgrade (convert code to use Python 3.9+ idioms)
  • isort (sort import statements)
  • black (format code)

Here's an example of how black will reformat the code in ATR.py:

-                self.TD[n] = self.atr[offset +
-                                        self.hasTA[n] +
-                                        self.hasTB[n] +
-                                        self.hasTC[n] +
-                                        self.hasTD[n]]
+                self.TD[n] = self.atr[
+                    offset
+                    + self.hasTA[n]
+                    + self.hasTB[n]
+                    + self.hasTC[n]
+                    + self.hasTD[n]
+                ]
 
-            self.interfaceBytesCount += self.hasTA[n] +\
-                self.hasTB[n] +\
-                self.hasTC[n] +\
-                self.hasTD[n]
+            self.interfaceBytesCount += (
+                self.hasTA[n] + self.hasTB[n] + self.hasTC[n] + self.hasTD[n]
+            )

Here's an example of how isort will sort imports in setup.py:

 import platform
 import shlex
 import subprocess
-from sysconfig import get_platform
 from shutil import which
+from sysconfig import get_platform
 
-from setuptools import setup, Extension
+from setuptools import Extension, setup
 from setuptools.command.build_py import build_py

Here's an example of how pyupgrade will update the code idioms in Exceptions.py:

     def __init__(self, message="", hresult=-1, *args):
-        super(SmartcardException, self).__init__(message, *args)
+        super().__init__(message, *args)
         self.hresult = int(hresult)
 
     def __str__(self):
-        text = super(SmartcardException, self).__str__()
+        text = super().__str__()
         if self.hresult != -1:
             hresult = self.hresult
             if hresult < 0:
                 # convert 0x-7FEFFFE3 into 0x8010001D
                 hresult += 0x100000000
-            text += ": {} (0x{:08X})".format(SCardGetErrorMessage(self.hresult), hresult)
+            text += f": {SCardGetErrorMessage(self.hresult)} (0x{hresult:08X})"

If this looks acceptable to you, I'll submit a PR that introduces a pre-commit configuration with these tools enabled.

I will strongly recommend enabling pre-commit.ci for this repo, which will have the added benefit that incoming PRs will automatically be checked for conformance with the code formatter rules. If the tools detect deviations, pre-commit.ci will update the PR with an additional commit that fixes the issue(s).

@LudovicRousseau
Copy link
Owner

I think that is a good idea.

Instead of submitting a HUGE PR I prefer if you provide the commands you used so I can run them on my side.

@kurtmckee
Copy link
Contributor Author

Rather than using commands, I'll submit a PR with a .pre-commit-config.yaml file and the isort config in pyproject.toml. You'll then be able to checkout the branch and run:

# Install pre-commit in your repo git hooks so it runs before git commits complete
pre-commit install

# Run all of the pre-commit hooks against all files in the repo
pre-commit run --all

I strongly recommend running that pre-commit run --all command several times until all of the hooks return success...sometimes pyupgrade needs to run a couple of times.

I work with pre-commit installed via pipx, but do you want it added as a development dependency? I can add that to dev-requirements.txt while I'm in there.

@kurtmckee
Copy link
Contributor Author

(I need to turn to work items now but I'll get that PR submitted when I have an opportunity!)

@kurtmckee
Copy link
Contributor Author

I've submitted #193, which introduces a pre-commit configuration.

I'm closing this issue as my question was answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants