GNOME.org
 

[damned-lies/technical-debt-reduction: 2/2] refactor: some technical debt reduction


commit fb96b02dd4d117bddf25b489f774177239cf92fd
Author: Guillaume Bernard <associations guillaume-bernard fr>
Date:   Mon May 3 21:10:58 2021 +0200

    refactor: some technical debt reduction
    
    - this mostly consists of updating unecesary if/elif/else condition
    blocks.
    - moves import on top of module files
    - replaces unused variables with an underscore

 api/views.py                       |  4 +--
 common/utils.py                    |  4 ++-
 languages/models.py                |  3 +-
 stats/doap.py                      |  2 +-
 stats/models.py                    | 34 +++++++++---------
 stats/potdiff.py                   |  4 +--
 stats/repos.py                     | 12 +++----
 stats/templatetags/stats_extras.py | 12 +++----
 stats/utils.py                     | 73 +++++++++++++++-----------------------
 stats/views.py                     |  6 ++--
 10 files changed, 68 insertions(+), 86 deletions(-)
---
diff --git a/api/views.py b/api/views.py
index 19cbf847..32fea9a7 100644
--- a/api/views.py
+++ b/api/views.py
@@ -216,7 +216,7 @@ class ModuleLangStatsView(VertimusPageMixin, View):
 @method_decorator(csrf_exempt, name='dispatch')
 class ReserveTranslationView(LoginRequiredMixin, VertimusPageMixin, View):
     def post(self, request, *args, **kwargs):
-        stats, state = self.get_state_from_kwargs()
+        _, state = self.get_state_from_kwargs()
         actions = [x.name for x in state.get_available_actions(request.user.person)]
         if 'RT' not in actions:
             return HttpResponseForbidden('This module cannot be reserved')
@@ -229,7 +229,7 @@ class ReserveTranslationView(LoginRequiredMixin, VertimusPageMixin, View):
 @method_decorator(csrf_exempt, name='dispatch')
 class UploadTranslationView(LoginRequiredMixin, VertimusPageMixin, View):
     def post(self, request, *args, **kwargs):
-        stats, state = self.get_state_from_kwargs()
+        _, state = self.get_state_from_kwargs()
         actions = [x.name for x in state.get_available_actions(request.user.person)]
         if 'UT' not in actions:
             return HttpResponseForbidden('You cannot upload a translation to this module')
diff --git a/common/utils.py b/common/utils.py
index d1c4bf3c..c39a4e5e 100644
--- a/common/utils.py
+++ b/common/utils.py
@@ -81,7 +81,9 @@ def is_site_admin(user):
 
 
 def get_user_locale(request):
-    from languages.models import Language  # Import here to prevent loop
+    # Import here to prevent loop import
+    # pylint: disable=import-outside-toplevel
+    from languages.models import Language
 
     curlang = Language.get_language_from_ianacode(request.LANGUAGE_CODE)
     if curlang and curlang.locale == 'en':
diff --git a/languages/models.py b/languages/models.py
index 6a6b12ee..acdce276 100644
--- a/languages/models.py
+++ b/languages/models.py
@@ -78,8 +78,9 @@ class Language(models.Model):
         }
 
     def get_release_stats(self, archives=False):
-        # FIXME Here be dragons
         """ Get summary stats for all releases """
+        # FIXME Here be dragons, prevent from cyclic import
+        # pylint disable=import-outside-toplevel
         from stats.models import Release
 
         if archives:
diff --git a/stats/doap.py b/stats/doap.py
index af20ca68..aa654768 100644
--- a/stats/doap.py
+++ b/stats/doap.py
@@ -62,7 +62,7 @@ def update_doap_infos(module):
         value = re.sub(r'[^\w\s-]', '', val).strip().lower()
         return re.sub(r'[-\s]+', '-', value)
     doap_maintainers = tree.parse_maintainers()
-    current_maintainers = dict([(m.email or m.username, m) for m in module.maintainers.all()])
+    current_maintainers = {m.email or m.username: m for m in module.maintainers.all()}
 
     # Using email or username as unique identifier
     for maintainer in doap_maintainers:
diff --git a/stats/models.py b/stats/models.py
index fef75a74..8ce759dd 100644
--- a/stats/models.py
+++ b/stats/models.py
@@ -26,6 +26,8 @@ from django.utils import dateformat
 from django.db import models
 from django.db.models.functions import Coalesce
 
+from translate.convert import sub2po
+
 from common.utils import is_site_admin, run_shell_command
 from stats import repos, signals, utils
 from stats.doap import update_doap_infos
@@ -801,7 +803,6 @@ class Domain(models.Model):
             pass  # Nothing to do, pot exists
 
         elif pot_method == 'subtitles':
-            from translate.convert import sub2po
             srt_files = [
                 p for p in vcs_path.iterdir() if p.is_file() and p.name.endswith('.srt')
             ]
@@ -920,24 +921,23 @@ class Domain(models.Model):
             # Custom (or no) linguas location
             if self.linguas_location == 'no':
                 return {'langs': None, 'error': ''}
-            elif self.linguas_location.split('/')[-1] == "LINGUAS":
+            if self.linguas_location.split('/')[-1] == "LINGUAS":
                 return utils.read_linguas_file(base_path / self.linguas_location)
+            variable = "ALL_LINGUAS"
+            if "#" in self.linguas_location:
+                file_path, variable = self.linguas_location.split("#")
             else:
-                variable = "ALL_LINGUAS"
-                if "#" in self.linguas_location:
-                    file_path, variable = self.linguas_location.split("#")
-                else:
-                    file_path = self.linguas_location
-                linguas_file = utils.MakefileWrapper(branch, base_path / file_path)
-                langs = linguas_file.read_variable(variable)
-                return {
-                    'langs': langs.split() if langs is not None else None,
-                    'error': gettext_noop(
-                        "Entry for this language is not present in %(var)s variable in %(file)s file." % {
-                            'var': variable, 'file': file_path
-                        }
-                    )
-                }
+                file_path = self.linguas_location
+            linguas_file = utils.MakefileWrapper(branch, base_path / file_path)
+            langs = linguas_file.read_variable(variable)
+            return {
+                'langs': langs.split() if langs is not None else None,
+                'error': gettext_noop(
+                    "Entry for this language is not present in %(var)s variable in %(file)s file." % {
+                        'var': variable, 'file': file_path
+                    }
+                )
+            }
         # Standard linguas location
         if self.dtype == 'ui':
             return utils.get_ui_linguas(branch, self.base_dir)
diff --git a/stats/potdiff.py b/stats/potdiff.py
index 63875b92..63266b69 100644
--- a/stats/potdiff.py
+++ b/stats/potdiff.py
@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 
 # Output differences between two POT files
+import difflib
 
 USE_DIFFLIB = 0
 
@@ -36,7 +37,6 @@ def diff(pota, potb):
                 j += 1
         return result_all, result_add_only
     else:
-        import difflib
         d = difflib.Differ()
         result = list(d.compare(res1, res2))
 
@@ -55,7 +55,7 @@ def _parse_contents(contents):
 
          [msgctxt::]msgid[/msgid_plural]"""
 
-    if len(contents) and contents[-1] != "\n":
+    if len(contents) > 0 and contents[-1] != "\n":
         contents += "\n"
 
     # state machine for parsing PO files
diff --git a/stats/repos.py b/stats/repos.py
index 8bbe1726..ca960bfe 100644
--- a/stats/repos.py
+++ b/stats/repos.py
@@ -20,8 +20,7 @@ class RepoBase:
         if self.exists():
             self.update()
             return
-        else:
-            self.init_checkout()
+        self.init_checkout()
 
     def init_checkout(self):
         raise NotImplementedError
@@ -44,7 +43,7 @@ class GitRepo(RepoBase):
         if not self.branch.co_path.exists():
             return False
         command = "git branch | grep %s" % self.branch.name
-        status, output, errs = run_shell_command(command, cwd=self.branch.co_path)
+        _, output, _ = run_shell_command(command, cwd=self.branch.co_path)
         return output != ""
 
     def init_checkout(self):
@@ -109,10 +108,9 @@ class GitRepo(RepoBase):
                 ['git', 'push', 'origin', self.branch.name], raise_on_error=True, cwd=commit_dir
             )
             return True
-        else:
-            # Revert
-            run_shell_command(['git', 'cherry-pick', '--abort'], cwd=commit_dir)
-            return False
+        # Revert
+        run_shell_command(['git', 'cherry-pick', '--abort'], cwd=commit_dir)
+        return False
 
     def remove(self):
         wdir = str(self.branch.co_path)
diff --git a/stats/templatetags/stats_extras.py b/stats/templatetags/stats_extras.py
index 9e07044a..0799982e 100644
--- a/stats/templatetags/stats_extras.py
+++ b/stats/templatetags/stats_extras.py
@@ -47,7 +47,7 @@ def support_class(value):
         Value is a translation percentage """
     if value >= 80:
         return "supported"
-    elif value >= 50:
+    if value >= 50:
         return "partially"
     return "not_supported"
 
@@ -59,11 +59,9 @@ def support_class_total(stats):
 
     if stats['diff'] >= 0:
         return support_class(actual)
-    else:
-        if actual >= 80:
-            return "partially"
-        else:
-            return "not_supported"
+    if actual >= 80:
+        return "partially"
+    return "not_supported"
 
 
 @register.filter
@@ -148,7 +146,7 @@ def num_stats_helper(stat, scope='full', strings=True):
 @register.filter
 def vis_stats(stat, scope='full'):
     """ Produce visual stats with green/red bar """
-    bidi = get_language_bidi() and "right" or "left"
+    bidi = "right" if get_language_bidi() else "left"
     if isinstance(stat, (Statistics,
                          FakeLangStatistics,
                          FakeSummaryStatistics)):
diff --git a/stats/utils.py b/stats/utils.py
index c76fcd4d..a80c74b3 100644
--- a/stats/utils.py
+++ b/stats/utils.py
@@ -36,7 +36,7 @@ orig_addunit = TranslationStore.addunit
 
 def patched_add_unit(self, unit):
     # Prevent two header units in the same store
-    if unit.isheader() and len(self.units) and self.units[0].isheader():
+    if unit.isheader() and len(self.units) > 0 and self.units[0].isheader():
         unit._store = self
         self.units[0] = unit
     else:
@@ -127,12 +127,11 @@ class DocFormat:
     def module_var(self):
         if self.use_meson:
             return "yelp.project_id"
-        elif self.tool == "itstool":
+        if self.tool == "itstool":
             return "HELP_ID"
-        elif self.format == "mallard":
+        if self.format == "mallard":
             return "DOC_ID"
-        else:
-            return "DOC_MODULE"
+        return "DOC_MODULE"
 
     @property
     def include_var(self):
@@ -140,26 +139,24 @@ class DocFormat:
             return "yelp.sources"
         if self.tool == "itstool":
             return "HELP_FILES"
-        elif self.format == "mallard":
+        if self.format == "mallard":
             return "DOC_PAGES"
-        else:
-            return "DOC_INCLUDES"
+        return "DOC_INCLUDES"
 
     @property
     def img_grep(self):
         if self.tool == "itstool":
             return "^msgid \"external ref="
-        else:
-            return "^msgid \"@@image:"
+        return "^msgid \"@@image:"
 
     @property
     def bef_line(self):
         # Lines to keep before matched grep to catch the ,fuzzy or #|msgid line
-        return self.tool == "itstool" and 2 or 1
+        return 2 if self.tool == "itstool" else 1
 
     @property
     def img_regex(self):
-        return self.tool == "itstool" and self.itstool_regex or self.xml2po_regex
+        return self.itstool_regex if self.tool == "itstool" else self.xml2po_regex
 
 
 class MakefileWrapper:
@@ -175,10 +172,9 @@ class MakefileWrapper:
                 if file_path.exists():
                     if file_name == 'meson.build':
                         return MesonfileWrapper(branch, file_path)
-                    elif file_name == 'CMakeLists.txt':
+                    if file_name == 'CMakeLists.txt':
                         return CMakefileWrapper(branch, file_path)
-                    else:
-                        return MakefileWrapper(branch, file_path)
+                    return MakefileWrapper(branch, file_path)
 
     def __init__(self, branch, path):
         self.branch = branch
@@ -320,8 +316,7 @@ class MesonfileWrapper(MakefileWrapper):
         def strip_mock(value):
             if isinstance(value, list):
                 return [v for v in value if not isinstance(v, MagicMock)]
-            else:
-                return value if not isinstance(value, MagicMock) else None
+            return value if not isinstance(value, MagicMock) else None
 
         for var in variables:
             if var in parsed_vars:
@@ -382,7 +377,7 @@ def ellipsize(val, length):
 
 def check_program_presence(prog_name):
     """ Test if prog_name is an available command on the system """
-    status, output, err = run_shell_command(['which', prog_name])
+    status, _, _ = run_shell_command(['which', prog_name])
     return status == 0
 
 
@@ -403,15 +398,6 @@ def po_grep(in_file, out_file, filter_):
             pogrep.rungrep(str(in_file), out, None, grepfilter)
         except Exception:
             pass
-    # command-line variant:
-    """
-    cmd = 'pogrep --invert-match --header --search=locations --regexp ' \
-          '"gschema\\.xml\\.in|schemas\\.in\" %(full_po)s %(part_po)s' % {
-        'full_po': in_file,
-        'part_po': out_file,
-    }
-    run_shell_command(cmd)
-    """
 
 
 def check_potfiles(po_path):
@@ -420,7 +406,7 @@ def check_potfiles(po_path):
     errors = []
 
     run_shell_command(['rm', '-f', 'missing', 'notexist'], cwd=po_path)
-    status, output, errs = run_shell_command(['intltool-update', '-m'], cwd=po_path)
+    status, _, _ = run_shell_command(['intltool-update', '-m'], cwd=po_path)
 
     if status != STATUS_OK:
         errors.append(("error", gettext_noop("Errors while running “intltool-update -m” check.")))
@@ -463,7 +449,7 @@ def generate_doc_pot_file(branch, domain):
     potbase = domain.potbase()
     potfile = doc_format.vcs_path / "C" / (potbase + ".pot")
     command = doc_format.command(potfile, files)
-    status, output, errs = run_shell_command(command, cwd=doc_format.vcs_path)
+    status, _, errs = run_shell_command(command, cwd=doc_format.vcs_path)
 
     if status != STATUS_OK:
         return "", [(
@@ -476,23 +462,21 @@ def generate_doc_pot_file(branch, domain):
 
     if not potfile.exists():
         return "", []
-    else:
-        return potfile, []
+    return potfile, []
 
 
 def pot_diff_status(pota, potb):
-    (status, output, errs) = run_shell_command('diff "%s" "%s"|wc -l' % (pota, potb))
+    _, output, _ = run_shell_command('diff "%s" "%s"|wc -l' % (pota, potb))
     # POT generation date always change and produce a 4 line diff
     if int(output) <= 4:
         return NOT_CHANGED, ""
 
     result_all, result_add_only = potdiff.diff(str(pota), str(potb))
-    if not len(result_all) and not len(result_add_only):
+    if len(result_all) == 0 and len(result_add_only) == 0:
         return CHANGED_ONLY_FORMATTING, ""
-    elif len(result_add_only):
+    if len(result_add_only) > 0:
         return CHANGED_WITH_ADDITIONS, result_add_only
-    else:
-        return CHANGED_NO_ADDITIONS, result_all
+    return CHANGED_NO_ADDITIONS, result_all
 
 
 def check_po_conformity(pofile):
@@ -550,8 +534,7 @@ def check_po_quality(pofile, filters):
     )
     if status == STATUS_OK:
         return out
-    else:
-        return _("Error running pofilter: %s") % errs
+    return _("Error running pofilter: %s") % errs
 
 
 def po_file_stats(pofile):
@@ -630,7 +613,7 @@ def get_ui_linguas(branch, subdir):
         if linguas.exists():
             return read_linguas_file(linguas)
     # AS_ALL_LINGUAS is a macro that takes all po files by default
-    status, output, errs = run_shell_command("grep -qs AS_ALL_LINGUAS %s%sconfigure.*" % (branch.co_path, 
os.sep))
+    status, _, _ = run_shell_command("grep -qs AS_ALL_LINGUAS %s%sconfigure.*" % (branch.co_path, os.sep))
     if status == 0:
         return {'langs': None,
                 'error': gettext_noop("No need to edit LINGUAS file or variable for this module")}
@@ -698,7 +681,7 @@ def get_fig_stats(pofile, doc_format):
     command = "msgcat --no-wrap %(pofile)s| grep -A 1 -B %(before)s '%(grep)s'" % {
         'pofile': pofile, 'grep': doc_format.img_grep, 'before': before_lines,
     }
-    (status, output, errs) = run_shell_command(command)
+    status, output, _ = run_shell_command(command)
     if status != STATUS_OK or output == '':
         # FIXME: something should be logged here
         return []
@@ -739,15 +722,15 @@ def add_custom_header(po_path, header, value):
     status = 1
     last_headers = ["Content-Transfer-Encoding", "Plural-Forms"]
     while status != 0 and last_headers != []:
-        (status, output, errs) = run_shell_command(grep_cmd)
+        status, output, _ = run_shell_command(grep_cmd)
         if status != 0:
             # Try to add header
             cmd = '''sed -i '/^\"%s/ a\\"%s: %s\\\\n"' %s''' % (last_headers.pop(), header, value, po_path)
-            (stat, out, err) = run_shell_command(cmd)
+            run_shell_command(cmd)
     if status == 0 and not "%s: %s" % (header, value) in output:
         # Set header value
         cmd = '''sed -i '/^\"%s/ c\\"%s: %s\\\\n"' %s''' % (header, header, value, po_path)
-        (stat, out, err) = run_shell_command(cmd)
+        run_shell_command(cmd)
 
 
 def exclude_untrans_messages(potfile):
@@ -772,8 +755,8 @@ def exclude_untrans_messages(potfile):
 
 
 def is_po_reduced(file_path):
-    status, output, errs = run_shell_command(['grep', 'X-DamnedLies-Scope: partial', file_path])
-    return (status == 0)
+    status, _, _ = run_shell_command(['grep', 'X-DamnedLies-Scope: partial', file_path])
+    return status == 0
 
 
 def compute_md5(full_path):
diff --git a/stats/views.py b/stats/views.py
index 1954226f..dd648b11 100644
--- a/stats/views.py
+++ b/stats/views.py
@@ -1,5 +1,6 @@
 from datetime import date
 import os
+from itertools import chain
 
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
@@ -192,7 +193,7 @@ def docimages(request, module_name, potbase, branch_name, langcode):
 def dynamic_po(request, module_name, branch_name, domain_name, filename):
     """ Generates a dynamic po file from the POT file of a branch """
     try:
-        locale, ext = filename.split(".")
+        locale, _ = filename.split(".")
         if locale.endswith('-reduced'):
             locale, reduced = locale[:-8], True
         else:
@@ -230,7 +231,7 @@ def dynamic_po(request, module_name, branch_name, domain_name, filename):
         dyn_content += "# FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.\n#\n"
 
     command = "msginit --locale=%s --no-translator --input=%s --output-file=-" % (locale, file_path)
-    status, output, err = run_shell_command(command, raise_on_error=True)
+    _, output, _ = run_shell_command(command, raise_on_error=True)
     lines = output.split("\n")
     skip_next_line = False
     for i, line in enumerate(lines):
@@ -265,7 +266,6 @@ def releases(request, format='html'):
     active_releases = Release.objects.filter(weight__gte=0).order_by('status', '-weight', '-name')
     old_releases = Release.objects.filter(weight__lt=0).order_by('status', '-weight', '-name')
     if format in ('json', 'xml'):
-        from itertools import chain
         data = serializers.serialize(format, chain(active_releases, old_releases))
         return HttpResponse(data, content_type=MIME_TYPES[format])
     context = {
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]