add notes about documentation and methods inside class

Because I always forget which way I used before

CONTRIBUTING.md

@@ -26,7 +26,95 @@ In order to resolve all difficult cases the `autopep8` is used. You can perform
 Again, the most checks can be performed by `make check` command, though some additional guidelines must be applied:
 
 * Every class, every function (including private and protected), every attribute must be documented. The project follows [Google style documentation](https://google.github.io/styleguide/pyguide.html). The only exception is local functions.
+* Correct way to document function, if section is empty, e.g. no notes or there are no args, it should be omitted:
+
+    ```python
+    def foo(argument: str, *, flag: bool = False) -> int:
+        """
+        do foo
+  
+        Note:
+            Very important note about this function
+  
+        Args:
+            argument(str): an argument
+            flag(bool, optional): a flag (Default value = False)
+  
+        Returns:
+            int: result
+  
+        Raises:
+            RuntimeException: a local function error occurs
+  
+        Examples:
+            Very informative example how to use this function, e.g.::
+  
+                >>> foo("argument", flag=False)
+  
+            Note that function documentation is in rST.
+        """
+    ```
+  
+  `Returns` should be replaced with `Yields` for generators.
+
+  Class attributes should be documented in the following way:
+
+    ```python
+    class Clazz(BaseClazz):
+        """
+        brand-new implementation of ``BaseClazz``
+  
+        Attributes:
+            CLAZZ_ATTRIBUTE(int): (class attribute) a brand-new class attribute
+            instance_attribute(str): an instance attribute
+  
+        Examples:
+            Very informative class usage example, e.g.::
+  
+                >>> from module import Clazz
+                >>> clazz = Clazz()
+        """
+  
+        CLAZZ_ATTRIBUTE = 42
+  
+        def __init__(self) -> None:
+            """
+            default constructor
+            """
+            self.instance_attribute = ""
+    ```
+
 * Type annotations are the must, even for local functions.
+* Recommended order of function definitions in class:
+
+    ```python
+    class Clazz:
+  
+        def __init__(self) -> None: ...  # replace with `__post_init__` for dataclasses
+
+        @property
+        def property(self) -> Any: ...
+
+        @classmethod
+        def class_method(cls: Type[Clazz]) -> Clazz: ...
+
+        @staticmethod
+        def static_method() -> Any: ...
+
+        def __private_method(self) -> Any: ...
+
+        def _protected_method(self) -> Any: ...
+
+        def usual_method(self) -> Any: ...
+
+        def __hash__(self) -> int: ...  # basically any magic (or look-alike) method
+    ```
+  
+  Methods inside one group should be ordered alphabetically, the only exception is `__init__` method (`__post__init__` for dataclasses) which should be defined first. For test methods it is recommended to follow the order in which functions are defined.
+
+  Though, we would like to highlight abstract methods (i.e. ones which raise `NotImplementedError`), we still keep in global order at the moment.
+
+* Abstract methods must raise `NotImplementedError` instead of using `abc.abstractmethod`. The reason behind this restriction is the fact that we have class/static abstract methods for those we need to define their attribute first making the code harder to read.
 * For any path interactions `pathlib.Path` must be used.
 * Configuration interactions must go through `ahriman.core.configuration.Configuration` class instance.
 * In case if class load requires some actions, it is recommended to create class method which can be used for class instantiating.
@@ -59,7 +147,7 @@ Again, the most checks can be performed by `make check` command, though some add
 * One file should define only one class, exception is class satellites in case if file length remains less than 400 lines.
 * It is possible to create file which contains some functions (e.g. `ahriman.core.util`), but in this case you would need to define `__all__` attribute.
 * The file size mentioned above must be applicable in general. In case of big classes consider splitting them into traits. Note, however, that `pylint` includes comments and docstrings into counter, thus you need to check file size by other tools.
-* No global variable is allowed outside of `ahriman.version` module.
+* No global variable is allowed outside of `ahriman.version` module. `ahriman.core.context` is also special case.
 * Single quotes are not allowed. The reason behind this restriction is the fact that docstrings must be written by using double quotes only, and we would like to make style consistent.
 * If your class writes anything to log, the `ahriman.core.log.LazyLogging` trait must be used.
 

src/ahriman/application/application/application_packages.py

@@ -37,30 +37,6 @@ class ApplicationPackages(ApplicationProperties):
     package control class
     """
 
-    def _known_packages(self) -> Set[str]:
-        """
-        load packages from repository and pacman repositories
-
-        Returns:
-            Set[str]: list of known packages
-
-        Raises:
-            NotImplementedError: not implemented method
-        """
-        raise NotImplementedError
-
-    def on_result(self, result: Result) -> None:
-        """
-        generate report and sync to remote server
-
-        Args:
-            result(Result): build result
-
-        Raises:
-            NotImplementedError: not implemented method
-        """
-        raise NotImplementedError
-
     def _add_archive(self, source: str, *_: Any) -> None:
         """
         add package from archive
@@ -147,6 +123,18 @@ class ApplicationPackages(ApplicationProperties):
         self.database.remote_update(package)
         # repository packages must not depend on unknown packages, thus we are not going to process dependencies
 
+    def _known_packages(self) -> Set[str]:
+        """
+        load packages from repository and pacman repositories
+
+        Returns:
+            Set[str]: list of known packages
+
+        Raises:
+            NotImplementedError: not implemented method
+        """
+        raise NotImplementedError
+
     def _process_dependencies(self, local_dir: Path, known_packages: Set[str], without_dependencies: bool) -> None:
         """
         process package dependencies
@@ -178,6 +166,18 @@ class ApplicationPackages(ApplicationProperties):
             fn = getattr(self, f"_add_{resolved_source.value}")
             fn(name, known_packages, without_dependencies)
 
+    def on_result(self, result: Result) -> None:
+        """
+        generate report and sync to remote server
+
+        Args:
+            result(Result): build result
+
+        Raises:
+            NotImplementedError: not implemented method
+        """
+        raise NotImplementedError
+
     def remove(self, names: Iterable[str]) -> None:
         """
         remove packages from repository

src/ahriman/application/application/application_repository.py

@@ -33,18 +33,6 @@ class ApplicationRepository(ApplicationProperties):
     repository control class
     """
 
-    def on_result(self, result: Result) -> None:
-        """
-        generate report and sync to remote server
-
-        Args:
-            result(Result): build result
-
-        Raises:
-            NotImplementedError: not implemented method
-        """
-        raise NotImplementedError
-
     def clean(self, *, cache: bool, chroot: bool, manual: bool, packages: bool, pacman: bool) -> None:
         """
         run all clean methods. Warning: some functions might not be available under non-root
@@ -67,6 +55,18 @@ class ApplicationRepository(ApplicationProperties):
         if pacman:
             self.repository.clear_pacman()
 
+    def on_result(self, result: Result) -> None:
+        """
+        generate report and sync to remote server
+
+        Args:
+            result(Result): build result
+
+        Raises:
+            NotImplementedError: not implemented method
+        """
+        raise NotImplementedError
+
     def sign(self, packages: Iterable[str]) -> None:
         """
         sign packages and repository

src/ahriman/application/handlers/versions.py

@@ -61,7 +61,7 @@ class Versions(Handler):
 
         Args:
             root(str): root package name
-            root_extras(Tuple[str, ...]): extras for the root package (Default value = ())
+            root_extras(Tuple[str, ...], optional): extras for the root package (Default value = ())
 
         Returns:
             Dict[str, str]: map of installed dependency to its version

src/ahriman/application/lock.py

@@ -75,40 +75,6 @@ class Lock(LazyLogging):
         self.paths = configuration.repository_paths
         self.reporter = Client.load(configuration, report=args.report)
 
-    def __enter__(self) -> Lock:
-        """
-        default workflow is the following:
-
-            1. Check user UID
-            2. Check if there is lock file
-            3. Check web status watcher status
-            4. Create lock file
-            5. Report to status page if enabled
-        """
-        self.check_user()
-        self.check_version()
-        self.create()
-        self.reporter.update_self(BuildStatusEnum.Building)
-        return self
-
-    def __exit__(self, exc_type: Optional[Type[Exception]], exc_val: Optional[Exception],
-                 exc_tb: TracebackType) -> Literal[False]:
-        """
-        remove lock file when done
-
-        Args:
-            exc_type(Optional[Type[Exception]]): exception type name if any
-            exc_val(Optional[Exception]): exception raised if any
-            exc_tb(TracebackType): exception traceback if any
-
-        Returns:
-            Literal[False]: always False (do not suppress any exception)
-        """
-        self.clear()
-        status = BuildStatusEnum.Success if exc_val is None else BuildStatusEnum.Failed
-        self.reporter.update_self(status)
-        return False
-
     def check_version(self) -> None:
         """
         check web server version
@@ -145,3 +111,37 @@ class Lock(LazyLogging):
             self.path.touch(exist_ok=self.force)
         except FileExistsError:
             raise DuplicateRunError()
+
+    def __enter__(self) -> Lock:
+        """
+        default workflow is the following:
+
+            1. Check user UID
+            2. Check if there is lock file
+            3. Check web status watcher status
+            4. Create lock file
+            5. Report to status page if enabled
+        """
+        self.check_user()
+        self.check_version()
+        self.create()
+        self.reporter.update_self(BuildStatusEnum.Building)
+        return self
+
+    def __exit__(self, exc_type: Optional[Type[Exception]], exc_val: Optional[Exception],
+                 exc_tb: TracebackType) -> Literal[False]:
+        """
+        remove lock file when done
+
+        Args:
+            exc_type(Optional[Type[Exception]]): exception type name if any
+            exc_val(Optional[Exception]): exception raised if any
+            exc_tb(TracebackType): exception traceback if any
+
+        Returns:
+            Literal[False]: always False (do not suppress any exception)
+        """
+        self.clear()
+        status = BuildStatusEnum.Success if exc_val is None else BuildStatusEnum.Failed
+        self.reporter.update_self(status)
+        return False

src/ahriman/core/alpm/pacman.py

@@ -81,25 +81,6 @@ class Pacman(LazyLogging):
 
         return handle
 
-    def __getattr__(self, item: str) -> Any:
-        """
-        pacman handle extractor
-
-        Args:
-            item(str): property name
-
-        Returns:
-            Any: attribute by its name
-
-        Raises:
-            AttributeError: in case if no such attribute found
-        """
-        if item == "handle":
-            handle = self.__create_handle_fn()
-            setattr(self, item, handle)
-            return handle
-        return super().__getattr__(item)  # required for logging attribute
-
     def database_copy(self, handle: Handle, database: DB, pacman_root: Path, paths: RepositoryPaths, *,
                       use_ahriman_cache: bool) -> None:
         """
@@ -198,3 +179,22 @@ class Pacman(LazyLogging):
                 result.update(package.provides)  # provides list for meta-packages
 
         return result
+
+    def __getattr__(self, item: str) -> Any:
+        """
+        pacman handle extractor
+
+        Args:
+            item(str): property name
+
+        Returns:
+            Any: attribute by its name
+
+        Raises:
+            AttributeError: in case if no such attribute found
+        """
+        if item == "handle":
+            handle = self.__create_handle_fn()
+            setattr(self, item, handle)
+            return handle
+        return super().__getattr__(item)  # required for logging attribute

src/ahriman/core/formatters/printer.py

@@ -33,8 +33,7 @@ class Printer:
 
         Args:
             verbose(bool): print all fields
-            log_fn(Callable[[str]): logger function to log data
-            None]:  (Default value = print)
+            log_fn(Callable[[str], None]): logger function to log data (Default value = print)
             separator(str, optional): separator for property name and property value (Default value = ": ")
         """
         if (title := self.title()) is not None:

src/ahriman/core/log/lazy_logging.py

@@ -33,25 +33,6 @@ class LazyLogging:
 
     logger: logging.Logger
 
-    def __getattr__(self, item: str) -> Any:
-        """
-        logger extractor
-
-        Args:
-            item(str): property name
-
-        Returns:
-            Any: attribute by its name
-
-        Raises:
-            AttributeError: in case if no such attribute found
-        """
-        if item == "logger":
-            logger = logging.getLogger(self.logger_name)
-            setattr(self, item, logger)
-            return logger
-        raise AttributeError(f"'{self.__class__.__qualname__}' object has no attribute '{item}'")
-
     @property
     def logger_name(self) -> str:
         """
@@ -107,3 +88,22 @@ class LazyLogging:
             yield
         finally:
             self._package_logger_reset()
+
+    def __getattr__(self, item: str) -> Any:
+        """
+        logger extractor
+
+        Args:
+            item(str): property name
+
+        Returns:
+            Any: attribute by its name
+
+        Raises:
+            AttributeError: in case if no such attribute found
+        """
+        if item == "logger":
+            logger = logging.getLogger(self.logger_name)
+            setattr(self, item, logger)
+            return logger
+        raise AttributeError(f"'{self.__class__.__qualname__}' object has no attribute '{item}'")

src/ahriman/core/repository/cleaner.py

@@ -30,18 +30,6 @@ class Cleaner(RepositoryProperties):
     trait to clean common repository objects
     """
 
-    def packages_built(self) -> List[Path]:
-        """
-        get list of files in built packages directory
-
-        Returns:
-            List[Path]: list of filenames from the directory
-
-        Raises:
-            NotImplementedError: not implemented method
-        """
-        raise NotImplementedError
-
     def clear_cache(self) -> None:
         """
         clear cache directory
@@ -80,3 +68,15 @@ class Cleaner(RepositoryProperties):
         """
         self.logger.info("clear build queue")
         self.database.build_queue_clear(None)
+
+    def packages_built(self) -> List[Path]:
+        """
+        get list of files in built packages directory
+
+        Returns:
+            List[Path]: list of filenames from the directory
+
+        Raises:
+            NotImplementedError: not implemented method
+        """
+        raise NotImplementedError

src/ahriman/core/triggers/trigger_loader.py

@@ -75,14 +75,6 @@ class TriggerLoader(LazyLogging):
             for trigger in configuration.getlist("build", "triggers")
         ]
 
-    def __del__(self) -> None:
-        """
-        custom destructor object which calls on_stop in case if it was requested
-        """
-        if not self._on_stop_requested:
-            return
-        self.on_stop()
-
     @contextlib.contextmanager
     def __execute_trigger(self, trigger: Trigger) -> Generator[None, None, None]:
         """
@@ -206,3 +198,11 @@ class TriggerLoader(LazyLogging):
         for trigger in self.triggers:
             with self.__execute_trigger(trigger):
                 trigger.on_stop()
+
+    def __del__(self) -> None:
+        """
+        custom destructor object which calls on_stop in case if it was requested
+        """
+        if not self._on_stop_requested:
+            return
+        self.on_stop()

tests/ahriman/application/application/test_application_packages.py

@@ -11,22 +11,6 @@ from ahriman.models.package_source import PackageSource
 from ahriman.models.result import Result
 
 
-def test_on_result(application_packages: ApplicationPackages) -> None:
-    """
-    must raise NotImplemented for missing finalize method
-    """
-    with pytest.raises(NotImplementedError):
-        application_packages.on_result(Result())
-
-
-def test_known_packages(application_packages: ApplicationPackages) -> None:
-    """
-    must raise NotImplemented for missing known_packages method
-    """
-    with pytest.raises(NotImplementedError):
-        application_packages._known_packages()
-
-
 def test_add_archive(
         application_packages: ApplicationPackages,
         package_ahriman: Package,
@@ -129,6 +113,14 @@ def test_add_repository(application_packages: ApplicationPackages, package_ahrim
     update_remote_mock.assert_called_once_with(package_ahriman)
 
 
+def test_known_packages(application_packages: ApplicationPackages) -> None:
+    """
+    must raise NotImplemented for missing known_packages method
+    """
+    with pytest.raises(NotImplementedError):
+        application_packages._known_packages()
+
+
 def test_process_dependencies(application_packages: ApplicationPackages, mocker: MockerFixture) -> None:
     """
     must process dependencies addition
@@ -237,6 +229,14 @@ def test_add_add_remote(application_packages: ApplicationPackages, package_descr
     add_mock.assert_called_once_with(url, set(), False)
 
 
+def test_on_result(application_packages: ApplicationPackages) -> None:
+    """
+    must raise NotImplemented for missing finalize method
+    """
+    with pytest.raises(NotImplementedError):
+        application_packages.on_result(Result())
+
+
 def test_remove(application_packages: ApplicationPackages, mocker: MockerFixture) -> None:
     """
     must remove package

tests/ahriman/application/application/test_application_repository.py

@@ -9,14 +9,6 @@ from ahriman.models.package import Package
 from ahriman.models.result import Result
 
 
-def test_on_result(application_repository: ApplicationRepository) -> None:
-    """
-    must raise NotImplemented for missing finalize method
-    """
-    with pytest.raises(NotImplementedError):
-        application_repository.on_result(Result())
-
-
 def test_clean_cache(application_repository: ApplicationRepository, mocker: MockerFixture) -> None:
     """
     must clean cache directory
@@ -62,6 +54,14 @@ def test_clean_pacman(application_repository: ApplicationRepository, mocker: Moc
     clear_mock.assert_called_once_with()
 
 
+def test_on_result(application_repository: ApplicationRepository) -> None:
+    """
+    must raise NotImplemented for missing finalize method
+    """
+    with pytest.raises(NotImplementedError):
+        application_repository.on_result(Result())
+
+
 def test_sign(application_repository: ApplicationRepository, package_ahriman: Package, package_python_schedule: Package,
               mocker: MockerFixture) -> None:
     """

tests/ahriman/application/test_lock.py

@@ -12,40 +12,6 @@ from ahriman.models.build_status import BuildStatus, BuildStatusEnum
 from ahriman.models.internal_status import InternalStatus
 
 
-def test_enter(lock: Lock, mocker: MockerFixture) -> None:
-    """
-    must process with context manager
-    """
-    check_user_mock = mocker.patch("ahriman.application.lock.Lock.check_user")
-    check_version_mock = mocker.patch("ahriman.application.lock.Lock.check_version")
-    clear_mock = mocker.patch("ahriman.application.lock.Lock.clear")
-    create_mock = mocker.patch("ahriman.application.lock.Lock.create")
-    update_status_mock = mocker.patch("ahriman.core.status.client.Client.update_self")
-
-    with lock:
-        pass
-    check_user_mock.assert_called_once_with()
-    clear_mock.assert_called_once_with()
-    create_mock.assert_called_once_with()
-    check_version_mock.assert_called_once_with()
-    update_status_mock.assert_has_calls([MockCall(BuildStatusEnum.Building), MockCall(BuildStatusEnum.Success)])
-
-
-def test_exit_with_exception(lock: Lock, mocker: MockerFixture) -> None:
-    """
-    must process with context manager in case if exception raised
-    """
-    mocker.patch("ahriman.application.lock.Lock.check_user")
-    mocker.patch("ahriman.application.lock.Lock.clear")
-    mocker.patch("ahriman.application.lock.Lock.create")
-    update_status_mock = mocker.patch("ahriman.core.status.client.Client.update_self")
-
-    with pytest.raises(Exception):
-        with lock:
-            raise Exception()
-    update_status_mock.assert_has_calls([MockCall(BuildStatusEnum.Building), MockCall(BuildStatusEnum.Failed)])
-
-
 def test_check_version(lock: Lock, mocker: MockerFixture) -> None:
     """
     must check version correctly
@@ -166,3 +132,37 @@ def test_create_unsafe(lock: Lock) -> None:
 
     lock.create()
     lock.path.unlink()
+
+
+def test_enter(lock: Lock, mocker: MockerFixture) -> None:
+    """
+    must process with context manager
+    """
+    check_user_mock = mocker.patch("ahriman.application.lock.Lock.check_user")
+    check_version_mock = mocker.patch("ahriman.application.lock.Lock.check_version")
+    clear_mock = mocker.patch("ahriman.application.lock.Lock.clear")
+    create_mock = mocker.patch("ahriman.application.lock.Lock.create")
+    update_status_mock = mocker.patch("ahriman.core.status.client.Client.update_self")
+
+    with lock:
+        pass
+    check_user_mock.assert_called_once_with()
+    clear_mock.assert_called_once_with()
+    create_mock.assert_called_once_with()
+    check_version_mock.assert_called_once_with()
+    update_status_mock.assert_has_calls([MockCall(BuildStatusEnum.Building), MockCall(BuildStatusEnum.Success)])
+
+
+def test_exit_with_exception(lock: Lock, mocker: MockerFixture) -> None:
+    """
+    must process with context manager in case if exception raised
+    """
+    mocker.patch("ahriman.application.lock.Lock.check_user")
+    mocker.patch("ahriman.application.lock.Lock.clear")
+    mocker.patch("ahriman.application.lock.Lock.create")
+    update_status_mock = mocker.patch("ahriman.core.status.client.Client.update_self")
+
+    with pytest.raises(Exception):
+        with lock:
+            raise Exception()
+    update_status_mock.assert_has_calls([MockCall(BuildStatusEnum.Building), MockCall(BuildStatusEnum.Failed)])

tests/ahriman/core/log/test_lazy_logging.py

@@ -8,22 +8,6 @@ from ahriman.core.database import SQLite
 from ahriman.models.package import Package
 
 
-def test_logger(database: SQLite) -> None:
-    """
-    must set logger attribute
-    """
-    assert database.logger
-    assert database.logger.name == "ahriman.core.database.sqlite.SQLite"
-
-
-def test_logger_attribute_error(database: SQLite) -> None:
-    """
-    must raise AttributeError in case if no attribute found
-    """
-    with pytest.raises(AttributeError):
-        database.loggerrrr
-
-
 def test_logger_name(database: SQLite, repo: Repo) -> None:
     """
     must correctly generate logger name
@@ -74,3 +58,19 @@ def test_in_package_context_failed(database: SQLite, package_ahriman: Package, m
             raise Exception()
 
     reset_mock.assert_called_once_with()
+
+
+def test_logger(database: SQLite) -> None:
+    """
+    must set logger attribute
+    """
+    assert database.logger
+    assert database.logger.name == "ahriman.core.database.sqlite.SQLite"
+
+
+def test_logger_attribute_error(database: SQLite) -> None:
+    """
+    must raise AttributeError in case if no attribute found
+    """
+    with pytest.raises(AttributeError):
+        database.loggerrrr

tests/ahriman/core/repository/test_cleaner.py

@@ -30,14 +30,6 @@ def _mock_clear_check() -> None:
     ])
 
 
-def test_packages_built(cleaner: Cleaner) -> None:
-    """
-    must raise NotImplemented for missing method
-    """
-    with pytest.raises(NotImplementedError):
-        cleaner.packages_built()
-
-
 def test_clear_cache(cleaner: Cleaner, mocker: MockerFixture) -> None:
     """
     must remove every cached sources
@@ -84,3 +76,11 @@ def test_clear_queue(cleaner: Cleaner, mocker: MockerFixture) -> None:
     clear_mock = mocker.patch("ahriman.core.database.SQLite.build_queue_clear")
     cleaner.clear_queue()
     clear_mock.assert_called_once_with(None)
+
+
+def test_packages_built(cleaner: Cleaner) -> None:
+    """
+    must raise NotImplemented for missing method
+    """
+    with pytest.raises(NotImplementedError):
+        cleaner.packages_built()