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

Improvement in code quality before the 0.9.x branch was forked #39

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Commits on Mar 30, 2024

  1. Configuration menu
    Copy the full SHA
    bd01503 View commit details
    Browse the repository at this point in the history

Commits on Mar 31, 2024

  1. [py27] add support to run Python 2.7 tests

    As long as we support Python 2.7 we have to support the different installation
    methods (virtualenv) for Python 3 and Python 2.  For this purpose, a Makefile is
    added with this patch, which can be removed later when Python 2.7 is no longer
    supported (in the main line).
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    248fb6c View commit details
    Browse the repository at this point in the history
  2. [code-clean] conversion of CRLF to LF line ending

    Some of the files in the repository have CRLF line ending other have LF ending.
    This patch normalizes to unix-style (LF)::
    
        find . -type f -not -path './.git*' | xargs sed -i 's/\r//g'
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    7dc01b8 View commit details
    Browse the repository at this point in the history
  3. [code-clean] initial code formatting (python black)

    Command to format the code [1]::
    
        python -m black .
    
    Black version::
    
        $ python -m black --version
        python -m black, 24.3.0 (compiled: yes)
        Python (CPython) 3.12.2
    
    HINT: Black removes some u'' strings that are Unicode in Python 2.x, these had
    to be corrected manually.
    
    [1] https://black.readthedocs.io/en/stable/#
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    3c62286 View commit details
    Browse the repository at this point in the history
  4. [code-clean] handle common issues reported by pylint

    This patch adds a pylint profile (.pylintrc) with the aim to have a good
    compromise in context of Python v2 & v3 and the status of the current
    implementation.
    
    To avoid having to switch off all checks in the profile, many of the issues
    reported by pylint were handled by using inline comments.
    
    Simple errors in the code were corrected, but only if the effect of the change
    was locally limited and manageable.
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    2b1c039 View commit details
    Browse the repository at this point in the history
  5. [code-clean] handle pylint messages from python2 compatibility

    This patch disables issues reported by pylint in py3 that we have from the
    py2 compatibility.
    
    HINT:
    
        When py2 compatibility is abandoned, this commit can be reverted and the
        code passages can be converted to Py3.
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    35e3ccf View commit details
    Browse the repository at this point in the history
  6. [code-clean] fix unidiomatic-typecheck reported by pylint (C0123)

    The idiomatic way to perform an explicit typecheck in Python is to use
    `isinstance(x, Y)` rather than `type(x) == Y`, `type(x) is Y`. [1]
    
    [1] https://pylint.pycqa.org/en/latest/user_guide/messages/convention/unidiomatic-typecheck.html
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    73a0fde View commit details
    Browse the repository at this point in the history
  7. [code-clean] The datetime class is a subclass of date.

    Since the class `datetime.datetime` is a subclass of `datetime.date`, the
    `isinstance` check is not suitable if a distinction has to be made between the
    two classes.
    
    The `type` check is more suitable in these cases, as no further inheritance can
    be assumed for these two object classes and the code is also easier to read in
    the context of this distinction.
    
    The situation is completely different with method:
    
    - icalendar.Trigger.transformFromNative()
    
    in this method (in the Trgger class) a distinction is made between a date and a
    time duration.  Since `datetime.date` is the base class, it is suitable for an
    `isinstance` check of a date object (which can be an instance of the
    `datetime.datetime` class as well as `datetime.date`).
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    a2df0fd View commit details
    Browse the repository at this point in the history
  8. [code-clean] remove leftover from commit b564a07

    The tests._test() function has been removed in commit b564a07 / remove
    obsolte code.
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    c9750dc View commit details
    Browse the repository at this point in the history
  9. [code-clean] Signature differs from overridden 'prettyPrint' method

    The base class `vobject.base.Component` defines method's prototype by:
    
        def prettyPrint(self, level=0, tabwidth=3):
            ...
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    ab97254 View commit details
    Browse the repository at this point in the history
  10. [code-clean] disable keyword-arg-before-vararg warning reported by py…

    …lint (W1113)
    
    1. When defining a keyword argument before variable positional arguments, one
    can end up in having multiple values passed for the aforementioned parameter in
    case the method is called with keyword arguments[1].
    
    2. In the methods:
    
    - icalender.VCalendar2_0.serialize
    - icalender.VTimezone.validate
    - icalender.VEvent.validate
    - icalender.VTodo.validate
    - icalender.VAlarm.validate
    - icalender.VAvailability.validate
    - icalender.Available.validate
    
    the "arguments-differ (W0221)" [2] message from pylint had to be disbaled.  This
    message is a direct consequence of the "keyword-arg-before-vararg (W1113)" issue
    already present in the base class.
    
    3. in method:
    
    - vcard.Photo.serialize
    
    the "signature-differs (W0222)" [3] message from pylint had to be disbaled (for same
    reason as mentioned in 2.)
    
    ---
    
    HINT:
      It is advisable to correct the parameter definitions in a follow-up PR.
    
    [1] https://pylint.pycqa.org/en/latest/user_guide/messages/warning/keyword-arg-before-vararg.html
    [2] https://pylint.pycqa.org/en/latest/user_guide/messages/warning/arguments-differ.html
    [3] https://pylint.pycqa.org/en/latest/user_guide/messages/warning/signature-differs.html
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Mar 31, 2024
    Configuration menu
    Copy the full SHA
    30f95ce View commit details
    Browse the repository at this point in the history

Commits on Apr 1, 2024

  1. [code-clean] add black and pylint checks to the github CI.

    Adding the checks to the github workflow for PRs ensures that future PRs cannot
    be merged before they have passed the quality gate.
    
    The tests can be carried out locally as follows (see README.md)::
    
        $ make format pylint test
    
    The github workflows do not support Python 2 versions, the py2 quality gate can
    therefore only be tested locally and requires a Python 2 runtime (the Makefile
    assumes asdf is installed / as described in the README.md)::
    
        $ make clean venv2.7 test pylint2.7
    
    CAUTION:
    
      There are no automated tests for Python 2 on github, the downward compatible
      0.9.x releases have to be tested locally !!!
    
    About tools of the quality gate
    ===============================
    
    - Tools such as the code formatting of black are version-dependent / new rules
      are implemented with each new version of the tool.  To ensure that the same
      rules always apply to the quality gate, the versions of the tools must be
      pinned::
    
        black ==24.3.0; python_version >= "3.8"  # (newest) Black up from py3.8
        pylint ==3.1.0; python_version >= "3.8"  # (newest) Pylint up from py3.8
        pylint ==1.9.5; python_version =="2.7"   # old Pylint in py2.7
    
    - Since we support Python versions that have reached their EOL (py2.7 & Py3.7)
      we cannot add the tools of the quality gate to every python version we want
      to support / see remarks about the tools below.
    
    CAUTION:
    
      Upgrading the version of one of these tools must always be done in a
      separate PR / could be automated by tools like Renovate or Dependabot.
    
    About Black
    ===========
    
    - Black is not supported in py2 and py3.7 support stopped in Black v23.7.0.
      Since the Black test is already executed in other Python versions, a further
      test in py3.7 can be omitted.
    
    - We use black started from current version 24.3.0
    
    About Pylint
    ============
    
    - py2.7: latest pylint version for py2 is pylint v1.9.5
    
    - py3.7: since the Python test is already executed in other Python versions, a
      further test in py3.7 can be omitted.
    
    Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
    return42 committed Apr 1, 2024
    Configuration menu
    Copy the full SHA
    9249b88 View commit details
    Browse the repository at this point in the history