archlinux / archinstall

Arch Linux installer - guided, templates etc.
GNU General Public License v3.0
5.95k stars 514 forks source link

Evaluate the use of Ruff for code formatting #1682

Open Torxed opened 1 year ago

Torxed commented 1 year ago

In light of https://github.com/charliermarsh/ruff/issues/1904#issuecomment-1470946322 I'm intrigued to evaluate Ruff as a code formatter.

It looks like it could combat some of my quarrels with https://github.com/archlinux/archinstall/pull/997 (Linked with issue https://github.com/archlinux/archinstall/issues/872).

Specifically the sentences:

I wanted a tool that formats things consistently with a set of rules that I can explain in prose, and recognize in the output.

YAPF caused its results to be sometimes hard to understand. No configuration file we tried saved us from very suboptimal edge cases

I just wanted a tool that does one thing and does it well.

I ran a super default test and it doesn't appear to be very intrusive. It can also be found in the WIP PR I opened: https://github.com/archlinux/archinstall/pull/1681

~/archinstall (eval-ruff)$ ruff check archinstall/*.py --fix
diff --git a/archinstall/__init__.py b/archinstall/__init__.py
index b22b466..b57b468 100644
--- a/archinstall/__init__.py
+++ b/archinstall/__init__.py
@@ -6,7 +6,6 @@ from .lib.disk import *
 from .lib.exceptions import *
 from .lib.general import *
 from .lib.hardware import *
-from .lib.installer import __packages__, Installer, accessibility_tools_in_use
 from .lib.locale_helpers import *
 from .lib.luks import *
 from .lib.mirrors import *
@@ -14,37 +13,14 @@ from .lib.models.network_configuration import NetworkConfigurationHandler
 from .lib.models.users import User
 from .lib.networking import *
 from .lib.output import *
-from .lib.models.dataclasses import (
-       VersionDef,
-       PackageSearchResult,
-       PackageSearch,
-       LocalPackage
-)
-from .lib.packages.packages import (
-       group_search,
-       package_search,
-       find_package,
-       find_packages,
-       installed_package,
-       validate_package_list,
-)
 from .lib.profiles import *
 from .lib.services import *
 from .lib.storage import *
 from .lib.systemd import *
 from .lib.user_interaction import *
-from .lib.menu import Menu
-from .lib.menu.list_manager import ListManager
-from .lib.menu.text_input import TextInput
-from .lib.menu.global_menu import GlobalMenu
-from .lib.menu.abstract_menu import (
-       Selector,
-       AbstractMenu
-)
 from .lib.translationhandler import TranslationHandler, DeferredTranslation
 from .lib.plugins import plugins, load_plugin # This initiates the plugin loading ceremony
 from .lib.configuration import *
-from .lib.udev import udevadm_info
 parser = ArgumentParser()

 __version__ = "2.5.3"
@@ -194,7 +170,6 @@ def load_config():
        """
        refine and set some arguments. Formerly at the scripts
        """
-       from .lib.models import NetworkConfiguration

        if (archinstall_lang := arguments.get('archinstall-language', None)) is not None:
                arguments['archinstall-language'] = TranslationHandler().get_language_by_name(archinstall_lang)
ambv commented 1 year ago

Ruff is a linter with automatic fixes. Autoformatting is a work-in-progress in it and I'm not sure it's available in any released version of Ruff yet.

From your output it should be obvious no formatting was performed.

Torxed commented 1 year ago

Ruff is a linter with automatic fixes. Autoformatting is a work-in-progress in it and I'm not sure it's available in any released version of Ruff yet.

From your output it should be obvious no formatting was performed.

Noted from the discussion in the mentioned issue, however I'm intreagued in that development and see where it leads :D

I was happy about the few warnings as well as the cleanup that it did. I'm probably the only one but I appreciate the non-intrusiveness :)

svartkanin commented 1 year ago

I think ruff is having different goals, at least in the current state, than an actual formatter. It's main purpose seems to be the actual combination of other linters in existence, as far as I can tell it wouldn't even replace mypy but only flake8.

I think the "formatting" you experienced isn't actual formatting but just aligning some things, it would most likely not help giving the code base a uniform format...

The point of intrusiveness at this point is most likely that an initial run will do a lot of changes as the code is very diverse but as it continues after that it would be smaller