From a58139832358206a7b3a1324b6c38303028baf57 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Mon, 22 May 2017 11:01:52 -0700 Subject: [PATCH 1/4] Try unpinning testing reqs --- readthedocs/integrations/models.py | 6 +++--- readthedocs/wsgi.py | 2 +- requirements/lint.txt | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/readthedocs/integrations/models.py b/readthedocs/integrations/models.py index 80d8d8a9342..dee414c5513 100644 --- a/readthedocs/integrations/models.py +++ b/readthedocs/integrations/models.py @@ -199,7 +199,7 @@ def get(self, *args, **kwargs): def subclass(self, instance): return self._get_subclass_replacement(instance) - def create(self, *args, **kwargs): # pylint: disable=unused-argument + def create(self, **kwargs): # pylint: disable=unused-argument """Override of create method to use subclass instance instead Instead of using the underlying Integration model to create this @@ -295,7 +295,7 @@ class GenericAPIWebhook(Integration): class Meta: proxy = True - def save(self, *args, **kwargs): + def save(self, **kwargs): """Ensure model has token data before saving""" try: token = self.provider_data.get('token') @@ -305,7 +305,7 @@ def save(self, *args, **kwargs): if token is None: token = default_token() self.provider_data = {'token': token} - super(GenericAPIWebhook, self).save(*args, **kwargs) + super(GenericAPIWebhook, self).save(**kwargs) @property def token(self): diff --git a/readthedocs/wsgi.py b/readthedocs/wsgi.py index 4e4b415c902..544675ce8e7 100644 --- a/readthedocs/wsgi.py +++ b/readthedocs/wsgi.py @@ -7,7 +7,7 @@ # This application object is used by any WSGI server configured to use this # file. This includes Django's development server, if the WSGI_APPLICATION # setting points here. -from django.core.wsgi import get_wsgi_application +from django.core.wsgi import get_wsgi_application # pylint: disable=wrong-import-position application = get_wsgi_application() # Apply WSGI middleware here. diff --git a/requirements/lint.txt b/requirements/lint.txt index 42c328c319c..fb39b4bdf50 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -1,8 +1,8 @@ -r pip.txt maxcdn -astroid<1.4 -pylint<1.5 +astroid +pylint # prospector==0.12.6 currently has issues with pydocstyle prospector==0.12.5 -pylint-django<0.7 -pyflakes<1.2.0 +pylint-django +pyflakes From fcaf401803d1de53dd55eebd615324ad6811ec55 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Tue, 13 Jun 2017 12:24:13 -0700 Subject: [PATCH 2/4] Bump up prospector higher level strictness --- prospector-more.yml | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/prospector-more.yml b/prospector-more.yml index 9930284f339..c116418b83a 100644 --- a/prospector-more.yml +++ b/prospector-more.yml @@ -1,6 +1,39 @@ inherits: prospector -strictness: medium +strictness: high + +ignore-paths: + - constants.py + - urls.py + - wsgi.py + - acl + - api + - betterversion + - bookmarks + - builds + - cdn + - comments + - core + - djangome + - doc_builder + - donate + - gold + - integrations + - locale + - notifications + - oauth + - payments + - privacy + - profiles + - projects + - redirects + - restapi + - rtd_tests + - search + - settings + - tastyapi + - templates + - vcs_support pylint: options: From 8474b02ede5dd7ca94dac615065b8db5d4f5d79b Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Tue, 13 Jun 2017 12:24:45 -0700 Subject: [PATCH 3/4] Linting issues: arguments-differ, len-as-condition, redefined variables --- readthedocs/api/utils.py | 2 +- readthedocs/bookmarks/views.py | 16 ++++++++-------- readthedocs/builds/forms.py | 4 ++-- readthedocs/builds/models.py | 4 ++-- readthedocs/comments/models.py | 4 ++-- readthedocs/comments/views.py | 2 +- readthedocs/core/forms.py | 6 +++--- .../core/management/commands/import_github.py | 2 +- readthedocs/core/management/commands/pull.py | 2 +- .../core/management/commands/update_repos.py | 2 +- readthedocs/core/resolver.py | 5 +---- readthedocs/core/symlink.py | 8 ++++---- readthedocs/doc_builder/backends/mkdocs.py | 6 +++--- readthedocs/doc_builder/backends/sphinx.py | 4 ++-- readthedocs/doc_builder/base.py | 2 +- readthedocs/doc_builder/environments.py | 2 +- readthedocs/integrations/models.py | 6 +++--- readthedocs/profiles/views.py | 6 +++--- readthedocs/projects/forms.py | 5 ++++- readthedocs/projects/models.py | 16 ++++++++-------- readthedocs/projects/tasks.py | 2 +- readthedocs/projects/utils.py | 4 ++-- readthedocs/projects/version_handling.py | 4 ++-- readthedocs/restapi/permissions.py | 4 ++-- readthedocs/search/parse_json.py | 2 +- readthedocs/vcs_support/backends/git.py | 2 +- readthedocs/vcs_support/backends/hg.py | 18 +++++++++--------- 27 files changed, 70 insertions(+), 70 deletions(-) diff --git a/readthedocs/api/utils.py b/readthedocs/api/utils.py index 7a75c5dab36..a829a522887 100644 --- a/readthedocs/api/utils.py +++ b/readthedocs/api/utils.py @@ -160,7 +160,7 @@ def is_authenticated(self, request, **kwargs): class EnhancedModelResource(ModelResource): - def obj_get_list(self, request=None, *_, **kwargs): + def obj_get_list(self, request=None, *_, **kwargs): # pylint: disable=arguments-differ """ A ORM-specific implementation of ``obj_get_list``. diff --git a/readthedocs/bookmarks/views.py b/readthedocs/bookmarks/views.py index 98425600815..0a40e0a57a0 100644 --- a/readthedocs/bookmarks/views.py +++ b/readthedocs/bookmarks/views.py @@ -24,8 +24,8 @@ class BookmarkExistsView(View): @method_decorator(csrf_exempt) - def dispatch(self, *args, **kwargs): - return super(BookmarkExistsView, self).dispatch(*args, **kwargs) + def dispatch(self, request, *args, **kwargs): + return super(BookmarkExistsView, self).dispatch(request, *args, **kwargs) def get(self, request): return HttpResponse( @@ -80,8 +80,8 @@ class BookmarkListView(ListView): model = Bookmark @method_decorator(login_required) - def dispatch(self, *args, **kwargs): - return super(BookmarkListView, self).dispatch(*args, **kwargs) + def dispatch(self, request, *args, **kwargs): + return super(BookmarkListView, self).dispatch(request, *args, **kwargs) def get_queryset(self): return Bookmark.objects.filter(user=self.request.user) @@ -93,8 +93,8 @@ class BookmarkAddView(View): @method_decorator(login_required) @method_decorator(csrf_exempt) - def dispatch(self, *args, **kwargs): - return super(BookmarkAddView, self).dispatch(*args, **kwargs) + def dispatch(self, request, *args, **kwargs): + return super(BookmarkAddView, self).dispatch(request, *args, **kwargs) def get(self, request): return HttpResponse( @@ -156,8 +156,8 @@ class BookmarkRemoveView(View): @method_decorator(login_required) @method_decorator(csrf_exempt) - def dispatch(self, *args, **kwargs): - return super(BookmarkRemoveView, self).dispatch(*args, **kwargs) + def dispatch(self, request, *args, **kwargs): + return super(BookmarkRemoveView, self).dispatch(request, *args, **kwargs) def get(self, request, *args, **kwargs): return render_to_response( diff --git a/readthedocs/builds/forms.py b/readthedocs/builds/forms.py index 0a306d4d423..3e4b4199aa5 100644 --- a/readthedocs/builds/forms.py +++ b/readthedocs/builds/forms.py @@ -31,8 +31,8 @@ class Meta: model = Version fields = ['active', 'privacy_level', 'tags'] - def save(self, *args, **kwargs): - obj = super(VersionForm, self).save(*args, **kwargs) + def save(self, commit=True): + obj = super(VersionForm, self).save(commit=commit) if obj.active and not obj.built and not obj.uploaded: trigger_build(project=obj.project, version=obj) return obj diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 5e1fdb967ff..bc219c5b564 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -144,7 +144,7 @@ def get_absolute_url(self): private = self.privacy_level == PRIVATE return self.project.get_docs_url(version_slug=self.slug, private=private) - def save(self, *args, **kwargs): + def save(self, *args, **kwargs): # pylint: disable=arguments-differ """Add permissions to the Version for all owners on save.""" from readthedocs.projects import tasks obj = super(Version, self).save(*args, **kwargs) @@ -157,7 +157,7 @@ def save(self, *args, **kwargs): broadcast(type='app', task=tasks.symlink_project, args=[self.project.pk]) return obj - def delete(self, *args, **kwargs): + def delete(self, *args, **kwargs): # pylint: disable=arguments-differ from readthedocs.projects import tasks log.info('Removing files for version %s', self.slug) tasks.clear_artifacts.delay(version_pk=self.pk) diff --git a/readthedocs/comments/models.py b/readthedocs/comments/models.py index d12e715978a..e13a2537ef7 100644 --- a/readthedocs/comments/models.py +++ b/readthedocs/comments/models.py @@ -32,12 +32,12 @@ def from_hash(self, version_slug, page, node_hash, project_slug=None): if project_slug: snapshots = snapshots.filter(node__project__slug=project_slug) - if len(snapshots) == 0: + if not snapshots.exists(): raise DocumentNode.DoesNotExist( "No node exists on %s with a current hash of %s" % ( page, node_hash)) - if len(snapshots) == 1: + if snapshots.count() == 1: # If we have found only one snapshot, we know we have the correct node. node = snapshots[0].node else: diff --git a/readthedocs/comments/views.py b/readthedocs/comments/views.py index 372cde12304..32421cda9e5 100644 --- a/readthedocs/comments/views.py +++ b/readthedocs/comments/views.py @@ -184,7 +184,7 @@ def get_queryset(self): return queryset @method_decorator(login_required) - def create(self, request): + def create(self, request, *args, **kwargs): project = Project.objects.get(slug=request.data['project']) comment = project.add_comment(version_slug=request.data['version'], page=request.data['document_page'], diff --git a/readthedocs/core/forms.py b/readthedocs/core/forms.py index 2d96a09788a..a5c9d079aaf 100644 --- a/readthedocs/core/forms.py +++ b/readthedocs/core/forms.py @@ -30,11 +30,11 @@ def __init__(self, *args, **kwargs): except AttributeError: pass - def save(self, *args, **kwargs): + def save(self, commit=True): first_name = self.cleaned_data.pop('first_name', None) last_name = self.cleaned_data.pop('last_name', None) - profile = super(UserProfileForm, self).save(*args, **kwargs) - if kwargs.get('commit', True): + profile = super(UserProfileForm, self).save(commit=commit) + if commit: user = profile.user user.first_name = first_name user.last_name = last_name diff --git a/readthedocs/core/management/commands/import_github.py b/readthedocs/core/management/commands/import_github.py index 45bb028e968..8ac31a37649 100644 --- a/readthedocs/core/management/commands/import_github.py +++ b/readthedocs/core/management/commands/import_github.py @@ -11,7 +11,7 @@ class Command(BaseCommand): help = __doc__ def handle(self, *args, **options): - if len(args): + if args: for slug in args: for service in GitHubService.for_user( User.objects.get(username=slug) diff --git a/readthedocs/core/management/commands/pull.py b/readthedocs/core/management/commands/pull.py index 391ddb8a2d6..d37e5c61b70 100644 --- a/readthedocs/core/management/commands/pull.py +++ b/readthedocs/core/management/commands/pull.py @@ -16,7 +16,7 @@ class Command(BaseCommand): help = __doc__ def handle(self, *args, **options): - if len(args): + if args: for slug in args: tasks.update_imported_docs( utils.version_from_slug(slug, LATEST).pk diff --git a/readthedocs/core/management/commands/update_repos.py b/readthedocs/core/management/commands/update_repos.py index 5492f70c04b..5aba20d580d 100644 --- a/readthedocs/core/management/commands/update_repos.py +++ b/readthedocs/core/management/commands/update_repos.py @@ -42,7 +42,7 @@ def handle(self, *args, **options): record = options['record'] force = options['force'] version = options['version'] - if len(args): + if args: for slug in args: if version and version != "all": log.info("Updating version %s for %s", version, slug) diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 66cbb85e4be..11d37180c4f 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -103,10 +103,7 @@ def resolve_path(self, project, filename='', version_slug=None, project_slug = project.slug subproject_slug = None - if project.single_version or single_version: - single_version = True - else: - single_version = False + single_version = bool(project.single_version or single_version) return self.base_resolve_path( project_slug=project_slug, diff --git a/readthedocs/core/symlink.py b/readthedocs/core/symlink.py index 583cbf8b46f..5570aa46d11 100644 --- a/readthedocs/core/symlink.py +++ b/readthedocs/core/symlink.py @@ -150,15 +150,15 @@ def symlink_cnames(self, domain=None): domains = [domain] else: domains = Domain.objects.filter(project=self.project) - for domain in domains: - self._log(u"Symlinking CNAME: {0} -> {1}".format(domain.domain, self.project.slug)) + for dom in domains: + self._log(u"Symlinking CNAME: {0} -> {1}".format(dom.domain, self.project.slug)) # CNAME to doc root - symlink = os.path.join(self.CNAME_ROOT, domain.domain) + symlink = os.path.join(self.CNAME_ROOT, dom.domain) run('ln -nsf {0} {1}'.format(self.project_root, symlink)) # Project symlink - project_cname_symlink = os.path.join(self.PROJECT_CNAME_ROOT, domain.domain) + project_cname_symlink = os.path.join(self.PROJECT_CNAME_ROOT, dom.domain) run('ln -nsf %s %s' % (self.project.doc_path, project_cname_symlink)) def remove_symlink_cname(self, domain): diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index 50c19b86682..173342c4794 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -149,7 +149,7 @@ def generate_dynamic_include(self): tmpl = template_loader.get_template('doc_builder/include.js.tmpl') return tmpl.render(include_ctx) - def build(self, **__): + def build(self): checkout_path = self.project.checkout_path(self.version.slug) build_command = [ 'python', @@ -180,7 +180,7 @@ class MkdocsJSON(BaseMkdocs): build_dir = '_build/json' use_theme = False - def build(self, **kwargs): + def build(self): user_config = yaml.safe_load( open(os.path.join(self.root_path, 'mkdocs.yml'), 'r') ) @@ -190,4 +190,4 @@ def build(self, **kwargs): user_config, open(os.path.join(self.root_path, 'mkdocs.yml'), 'w') ) - super(MkdocsJSON, self).build(**kwargs) + super(MkdocsJSON, self).build() diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index 3eaf558fcdb..db5d09605ab 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -137,7 +137,7 @@ def append_conf(self, **__): cwd=self.project.checkout_path(self.version.slug), ) - def build(self, **__): + def build(self): self.clean() project = self.project build_command = [ @@ -275,7 +275,7 @@ class PdfBuilder(BaseSphinx): sphinx_build_dir = '_build/latex' pdf_file_name = None - def build(self, **__): + def build(self): self.clean() cwd = self.project.conf_dir(self.version.slug) diff --git a/readthedocs/doc_builder/base.py b/readthedocs/doc_builder/base.py index 5e49f73e189..32387265230 100644 --- a/readthedocs/doc_builder/base.py +++ b/readthedocs/doc_builder/base.py @@ -48,7 +48,7 @@ def force(self, **__): log.info("Forcing a build") self._force = True - def build(self, id=None, **__): # pylint: disable=redefined-builtin + def build(self): """Do the actual building of the documentation.""" raise NotImplementedError diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index 0753d8b010d..b890ca77230 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -396,7 +396,7 @@ def update_build(self, state=None): if self.failure and isinstance(self.failure, BuildEnvironmentException): self.build['exit_code'] = self.failure.status_code - elif len(self.commands) > 0: + elif self.commands: self.build['exit_code'] = max([cmd.exit_code for cmd in self.commands]) diff --git a/readthedocs/integrations/models.py b/readthedocs/integrations/models.py index dee414c5513..e5262c145b5 100644 --- a/readthedocs/integrations/models.py +++ b/readthedocs/integrations/models.py @@ -199,7 +199,7 @@ def get(self, *args, **kwargs): def subclass(self, instance): return self._get_subclass_replacement(instance) - def create(self, **kwargs): # pylint: disable=unused-argument + def create(self, **kwargs): """Override of create method to use subclass instance instead Instead of using the underlying Integration model to create this @@ -295,7 +295,7 @@ class GenericAPIWebhook(Integration): class Meta: proxy = True - def save(self, **kwargs): + def save(self, *args, **kwargs): # pylint: disable=arguments-differ """Ensure model has token data before saving""" try: token = self.provider_data.get('token') @@ -305,7 +305,7 @@ def save(self, **kwargs): if token is None: token = default_token() self.provider_data = {'token': token} - super(GenericAPIWebhook, self).save(**kwargs) + super(GenericAPIWebhook, self).save(*args, **kwargs) @property def token(self): diff --git a/readthedocs/profiles/views.py b/readthedocs/profiles/views.py index 53591a0b5d3..14b9e8989bb 100644 --- a/readthedocs/profiles/views.py +++ b/readthedocs/profiles/views.py @@ -95,7 +95,7 @@ def create_profile(request, form_class, success_url=None, extra_context = {} context = RequestContext(request) for key, value in extra_context.items(): - context[key] = callable(value) and value() or value + context[key] = (value() if callable(value) else value) return render_to_response(template_name, {'form': form}, @@ -172,7 +172,7 @@ def edit_profile(request, form_class, success_url=None, extra_context = {} context = RequestContext(request) for key, value in extra_context.items(): - context[key] = callable(value) and value() or value + context[key] = (value() if callable(value) else value) return render_to_response(template_name, { 'form': form, @@ -244,7 +244,7 @@ def profile_detail(request, username, public_profile_field=None, extra_context = {} context = RequestContext(request) for key, value in extra_context.items(): - context[key] = callable(value) and value() or value + context[key] = (value() if callable(value) else value) return render_to_response(template_name, {'profile': profile_obj}, diff --git a/readthedocs/projects/forms.py b/readthedocs/projects/forms.py index 09fb5551da3..45da44af1e2 100644 --- a/readthedocs/projects/forms.py +++ b/readthedocs/projects/forms.py @@ -504,7 +504,10 @@ def __init__(self, *args, **kwargs): self.project = kwargs.pop('project', None) super(RedirectForm, self).__init__(*args, **kwargs) - def save(self, **_): + def save(self, **_): # pylint: disable=arguments-differ + # TODO this should respect the unused argument `commit`. It's not clear + # why this needs to be a call to `create`, instead of relying on the + # super `save()` call. redirect = Redirect.objects.create( project=self.project, redirect_type=self.cleaned_data['redirect_type'], diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 69ded120134..90eef739df8 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -61,7 +61,7 @@ class ProjectRelationship(models.Model): def __unicode__(self): return "%s -> %s" % (self.parent, self.child) - def save(self, *args, **kwargs): + def save(self, *args, **kwargs): # pylint: disable=arguments-differ if not self.alias: self.alias = self.child.slug super(ProjectRelationship, self).save(*args, **kwargs) @@ -298,7 +298,7 @@ def sync_supported_versions(self): verbose_name__in=supported).update(supported=False) self.versions.filter(verbose_name=LATEST_VERBOSE_NAME).update(supported=True) - def save(self, *args, **kwargs): + def save(self, *args, **kwargs): # pylint: disable=arguments-differ from readthedocs.projects import tasks first_save = self.pk is None if not self.slug: @@ -612,8 +612,8 @@ def find(self, filename, version): """ matches = [] for root, __, filenames in os.walk(self.full_doc_path(version)): - for filename in fnmatch.filter(filenames, filename): - matches.append(os.path.join(root, filename)) + for match in fnmatch.filter(filenames, filename): + matches.append(os.path.join(root, match)) return matches def full_find(self, filename, version): @@ -624,8 +624,8 @@ def full_find(self, filename, version): """ matches = [] for root, __, filenames in os.walk(self.checkout_path(version)): - for filename in fnmatch.filter(filenames, filename): - matches.append(os.path.join(root, filename)) + for match in fnmatch.filter(filenames, filename): + matches.append(os.path.join(root, match)) return matches def get_latest_build(self, finished=True): @@ -907,7 +907,7 @@ class Meta: def __unicode__(self): return "{domain} pointed at {project}".format(domain=self.domain, project=self.project.name) - def save(self, *args, **kwargs): + def save(self, *args, **kwargs): # pylint: disable=arguments-differ from readthedocs.projects import tasks parsed = urlparse(self.domain) if parsed.scheme or parsed.netloc: @@ -917,7 +917,7 @@ def save(self, *args, **kwargs): super(Domain, self).save(*args, **kwargs) broadcast(type='app', task=tasks.symlink_domain, args=[self.project.pk, self.pk]) - def delete(self, *args, **kwargs): + def delete(self, *args, **kwargs): # pylint: disable=arguments-differ from readthedocs.projects import tasks broadcast(type='app', task=tasks.symlink_domain, args=[self.project.pk, self.pk, True]) super(Domain, self).delete(*args, **kwargs) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 44b81ae2fd8..8175eafc604 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -109,7 +109,7 @@ def _log(self, msg): def run(self, pk, version_pk=None, build_pk=None, record=True, docker=False, search=True, force=False, localmedia=True, **__): - + # pylint: disable=arguments-differ self.project = self.get_project(pk) self.version = self.get_version(self.project, version_pk) self.build = self.get_build(build_pk) diff --git a/readthedocs/projects/utils.py b/readthedocs/projects/utils.py index 2c0b33d4a40..4a108472801 100644 --- a/readthedocs/projects/utils.py +++ b/readthedocs/projects/utils.py @@ -41,8 +41,8 @@ def find_file(filename): """ matches = [] for root, __, filenames in os.walk('.'): - for filename in fnmatch.filter(filenames, filename): - matches.append(os.path.join(root, filename)) + for match in fnmatch.filter(filenames, filename): + matches.append(os.path.join(root, match)) return matches diff --git a/readthedocs/projects/version_handling.py b/readthedocs/projects/version_handling.py index f2225a9a11d..293bea6904a 100644 --- a/readthedocs/projects/version_handling.py +++ b/readthedocs/projects/version_handling.py @@ -36,7 +36,7 @@ def prune_major(self, num_latest): all_keys = sorted(set(self._state.keys())) major_keep = [] for __ in range(num_latest): - if len(all_keys) > 0: + if all_keys: major_keep.append(all_keys.pop(-1)) for to_remove in all_keys: del self._state[to_remove] @@ -46,7 +46,7 @@ def prune_minor(self, num_latest): all_keys = sorted(set(minors.keys())) minor_keep = [] for __ in range(num_latest): - if len(all_keys) > 0: + if all_keys: minor_keep.append(all_keys.pop(-1)) for to_remove in all_keys: del self._state[major][to_remove] diff --git a/readthedocs/restapi/permissions.py b/readthedocs/restapi/permissions.py index 95664537f22..4f5cc066869 100644 --- a/readthedocs/restapi/permissions.py +++ b/readthedocs/restapi/permissions.py @@ -14,11 +14,11 @@ def has_object_permission(self, request, view, obj): class CommentModeratorOrReadOnly(permissions.BasePermission): - def has_object_permission(self, request, view, comment): + def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: return True # TODO: Similar logic to #1084 else: - return AdminPermission.is_admin(request.user, comment.node.project) + return AdminPermission.is_admin(request.user, obj.node.project) class RelatedProjectIsOwner(permissions.BasePermission): diff --git a/readthedocs/search/parse_json.py b/readthedocs/search/parse_json.py index 08189614754..d0f67bee3e7 100644 --- a/readthedocs/search/parse_json.py +++ b/readthedocs/search/parse_json.py @@ -106,7 +106,7 @@ def process_file(filename): else: log.info('Unable to index file due to no name %s', filename) return None - if 'body' in data and len(data['body']): + if 'body' in data and data['body']: body = PyQuery(data['body']) body_content = body.text().replace(u'ΒΆ', '') sections.extend(generate_sections_from_pyquery(body)) diff --git a/readthedocs/vcs_support/backends/git.py b/readthedocs/vcs_support/backends/git.py index 4f670605bc6..b5dad59d79e 100644 --- a/readthedocs/vcs_support/backends/git.py +++ b/readthedocs/vcs_support/backends/git.py @@ -151,7 +151,7 @@ def parse_branches(self, data): for branch in raw_branches: branch = filter(lambda f: f != '' and f != '*', branch) # Handle empty branches - if len(branch): + if branch: branch = branch[0] if branch.startswith('origin/'): cut_len = len('origin/') diff --git a/readthedocs/vcs_support/backends/hg.py b/readthedocs/vcs_support/backends/hg.py index 5370392f2a1..144352b6185 100644 --- a/readthedocs/vcs_support/backends/hg.py +++ b/readthedocs/vcs_support/backends/hg.py @@ -20,19 +20,19 @@ def update(self): return self.clone() def pull(self): - pull_output = self.run('hg', 'pull') - if pull_output[0] != 0: + (pull_retcode, _, _) = self.run('hg', 'pull') + if pull_retcode != 0: raise ProjectImportError( - ("Failed to get code from '%s' (hg pull): %s" - % (self.repo_url, pull_output[0])) + ("Failed to pull code from '%s': retcode=%s" + % (self.repo_url, pull_retcode)) ) - update_output = self.run('hg', 'update', '-C')[0] - if update_output[0] != 0: + (update_retcode, stdout, stderr) = self.run('hg', 'update', '-C') + if update_retcode != 0: raise ProjectImportError( - ("Failed to get code from '%s' (hg update): %s" - % (self.repo_url, pull_output[0])) + ("Failed to update code from '%s': retcode=%s" + % (self.repo_url, update_retcode)) ) - return update_output + return (update_retcode, stdout, stderr) def clone(self): self.make_clean_working_dir() From 0ab73a39f342e5e90bf2a2d8cbcaa544ce9a4b5d Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Tue, 13 Jun 2017 12:51:07 -0700 Subject: [PATCH 4/4] Remove unnecessary elses This addresses no-else-return, which removes a `return` in an `else` after an `if` that already has a `return`. This makes the default return more obvious in most cases. --- readthedocs/bookmarks/views.py | 49 ++++++++++++------------ readthedocs/builds/models.py | 3 +- readthedocs/comments/models.py | 23 +++++------ readthedocs/core/resolver.py | 6 +-- readthedocs/core/utils/__init__.py | 8 ++-- readthedocs/core/views/__init__.py | 5 +-- readthedocs/core/views/hooks.py | 16 +++----- readthedocs/doc_builder/config.py | 31 ++++++--------- readthedocs/doc_builder/environments.py | 3 +- readthedocs/oauth/utils.py | 17 ++++---- readthedocs/privacy/backend.py | 12 ++---- readthedocs/projects/models.py | 11 ++---- readthedocs/projects/version_handling.py | 6 +-- readthedocs/projects/views/public.py | 3 +- readthedocs/redirects/models.py | 3 +- readthedocs/restapi/permissions.py | 3 +- readthedocs/restapi/utils.py | 3 +- readthedocs/rtd_tests/mocks/mock_api.py | 3 +- readthedocs/search/parse_json.py | 3 +- readthedocs/search/utils.py | 3 +- readthedocs/vcs_support/backends/bzr.py | 3 +- readthedocs/vcs_support/backends/hg.py | 8 ++-- 22 files changed, 90 insertions(+), 132 deletions(-) diff --git a/readthedocs/bookmarks/views.py b/readthedocs/bookmarks/views.py index 0a40e0a57a0..65c1346f890 100644 --- a/readthedocs/bookmarks/views.py +++ b/readthedocs/bookmarks/views.py @@ -175,30 +175,29 @@ def post(self, request, *args, **kwargs): bookmark = get_object_or_404(Bookmark, pk=kwargs['bookmark_pk']) bookmark.delete() return HttpResponseRedirect(reverse('bookmark_list')) - else: - try: - post_json = json.loads(request.body) - project = Project.objects.get(slug=post_json['project']) - version = project.versions.get(slug=post_json['version']) - url = post_json['url'] - page = post_json['page'] - except KeyError: - return HttpResponseBadRequest( - json.dumps({'error': "Invalid parameters"}) - ) - - bookmark = get_object_or_404( - Bookmark, - user=request.user, - url=url, - project=project, - version=version, - page=page + try: + post_json = json.loads(request.body) + project = Project.objects.get(slug=post_json['project']) + version = project.versions.get(slug=post_json['version']) + url = post_json['url'] + page = post_json['page'] + except KeyError: + return HttpResponseBadRequest( + json.dumps({'error': "Invalid parameters"}) ) - bookmark.delete() - return HttpResponse( - json.dumps({'removed': True}), - status=200, - content_type="application/json" - ) + bookmark = get_object_or_404( + Bookmark, + user=request.user, + url=url, + project=project, + version=version, + page=page + ) + bookmark.delete() + + return HttpResponse( + json.dumps({'removed': True}), + status=200, + content_type="application/json" + ) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index bc219c5b564..399ff3310ae 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -106,8 +106,7 @@ def commit_name(self): if self.slug == LATEST: if self.project.default_branch: return self.project.default_branch - else: - return self.project.vcs_repo().fallback_branch + return self.project.vcs_repo().fallback_branch if self.slug == STABLE: if self.type == BRANCH: diff --git a/readthedocs/comments/models.py b/readthedocs/comments/models.py index e13a2537ef7..9a3ee1eff03 100644 --- a/readthedocs/comments/models.py +++ b/readthedocs/comments/models.py @@ -84,22 +84,20 @@ def latest_commit(self): def visible_comments(self): if not self.project.comment_moderation: return self.comments.all() - else: - # non-optimal SQL warning. - decisions = ModerationAction.objects.filter( - comment__node=self, - decision=1, - date__gt=self.snapshots.latest().date - ) - valid_comments = self.comments.filter(moderation_actions__in=decisions).distinct() - return valid_comments + # non-optimal SQL warning. + decisions = ModerationAction.objects.filter( + comment__node=self, + decision=1, + date__gt=self.snapshots.latest().date + ) + valid_comments = self.comments.filter(moderation_actions__in=decisions).distinct() + return valid_comments def update_hash(self, new_hash, commit): latest_snapshot = self.snapshots.latest() if latest_snapshot.hash == new_hash and latest_snapshot.commit == commit: return latest_snapshot - else: - return self.snapshots.create(hash=new_hash, commit=commit) + return self.snapshots.create(hash=new_hash, commit=commit) class DocumentNodeSerializer(serializers.ModelSerializer): @@ -183,8 +181,7 @@ def has_been_approved_since_most_recent_node_change(self): # If we do have an approval action which is newer than the most recent change, # we'll return True or False commensurate with its "approved" attribute. return latest_moderation_action.approved() - else: - return False + return False def is_orphaned(self): raise NotImplementedError('TODO') diff --git a/readthedocs/core/resolver.py b/readthedocs/core/resolver.py index 11d37180c4f..4f80aca0b8a 100644 --- a/readthedocs/core/resolver.py +++ b/readthedocs/core/resolver.py @@ -125,8 +125,7 @@ def resolve_domain(self, project, private=None): return domain.domain elif self._use_subdomain(): return self._get_project_subdomain(canonical_project) - else: - return getattr(settings, 'PRODUCTION_DOMAIN') + return getattr(settings, 'PRODUCTION_DOMAIN') def resolve(self, project, protocol='http', filename='', private=None, **kwargs): @@ -155,8 +154,7 @@ def _get_canonical_project(self, project): return main_language_project elif relation: return relation.parent - else: - return project + return project def _get_project_subdomain(self, project): """Determine canonical project domain as subdomain""" diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index ae34fd032e3..4120d6e1c91 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -33,9 +33,8 @@ def run_on_app_servers(command): if ret != 0: ret_val = ret return ret_val - else: - ret = os.system(command) - return ret + ret = os.system(command) + return ret def broadcast(type, task, args): # pylint: disable=redefined-builtin @@ -56,8 +55,7 @@ def clean_url(url): parsed = urlparse(url) if parsed.scheme or parsed.netloc: return parsed.netloc - else: - return parsed.path + return parsed.path def cname_to_slug(host): diff --git a/readthedocs/core/views/__init__.py b/readthedocs/core/views/__init__.py index 8cf4ea3d7ff..ed72a247243 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -95,9 +95,8 @@ def wipe_version(request, project_slug, version_slug): for del_dir in del_dirs: broadcast(type='build', task=remove_dir, args=[del_dir]) return redirect('project_version_list', project_slug) - else: - return render_to_response('wipe_version.html', - context_instance=RequestContext(request)) + return render_to_response('wipe_version.html', + context_instance=RequestContext(request)) def divide_by_zero(request): # pylint: disable=unused-argument diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 4baf9ff2588..b6623c987de 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -220,11 +220,9 @@ def gitlab_build(request): # noqa: D205 projects = get_project_from_url(search_url) if projects: return _build_url(search_url, projects, branches) - else: - log.error('Project match not found: url=%s', search_url) - return HttpResponseNotFound('Project match not found') - else: - return HttpResponse('Method not allowed, POST is required', status=405) + log.error('Project match not found: url=%s', search_url) + return HttpResponseNotFound('Project match not found') + return HttpResponse('Method not allowed, POST is required', status=405) @csrf_exempt @@ -290,11 +288,9 @@ def bitbucket_build(request): branches ) return HttpResponseNotFound('Commit/branch not found') - else: - log.error('Project match not found: url=%s', search_url) - return HttpResponseNotFound('Project match not found') - else: - return HttpResponse('Method not allowed, POST is required', status=405) + log.error('Project match not found: url=%s', search_url) + return HttpResponseNotFound('Project match not found') + return HttpResponse('Method not allowed, POST is required', status=405) @csrf_exempt diff --git a/readthedocs/doc_builder/config.py b/readthedocs/doc_builder/config.py index a8c1c500e34..70d1f2e9c02 100644 --- a/readthedocs/doc_builder/config.py +++ b/readthedocs/doc_builder/config.py @@ -29,8 +29,7 @@ def __init__(self, version, yaml_config): def pip_install(self): if 'pip_install' in self._yaml_config.get('python', {}): return self._yaml_config['python']['pip_install'] - else: - return False + return False @property def install_project(self): @@ -38,16 +37,14 @@ def install_project(self): return True if 'setup_py_install' in self._yaml_config.get('python', {}): return self._yaml_config['python']['setup_py_install'] - else: - return self._project.install_project + return self._project.install_project @property def extra_requirements(self): if self.pip_install and 'extra_requirements' in self._yaml_config.get( 'python', {}): return self._yaml_config['python']['extra_requirements'] - else: - return [] + return [] @property def python_interpreter(self): @@ -77,8 +74,7 @@ def python_version(self): def use_system_site_packages(self): if 'use_system_site_packages' in self._yaml_config.get('python', {}): return self._yaml_config['python']['use_system_site_packages'] - else: - return self._project.use_system_packages + return self._project.use_system_packages @property def use_conda(self): @@ -88,27 +84,24 @@ def use_conda(self): def conda_file(self): if 'file' in self._yaml_config.get('conda', {}): return self._yaml_config['conda']['file'] - else: - return None + return None @property def requirements_file(self): if 'requirements_file' in self._yaml_config: return self._yaml_config['requirements_file'] - else: - return self._project.requirements_file + return self._project.requirements_file @property def formats(self): if 'formats' in self._yaml_config: return self._yaml_config['formats'] - else: - formats = ['htmlzip'] - if self._project.enable_epub_build: - formats += ['epub'] - if self._project.enable_pdf_build: - formats += ['pdf'] - return formats + formats = ['htmlzip'] + if self._project.enable_epub_build: + formats += ['epub'] + if self._project.enable_pdf_build: + formats += ['pdf'] + return formats # Not implemented until we figure out how to keep in sync with the webs. # Probably needs to be version-specific as well, not project. diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index b890ca77230..d64c8500fd2 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -165,8 +165,7 @@ def get_command(self): """Flatten command""" if hasattr(self.command, '__iter__') and not isinstance(self.command, str): return ' '.join(self.command) - else: - return self.command + return self.command def save(self): """Save this command and result via the API""" diff --git a/readthedocs/oauth/utils.py b/readthedocs/oauth/utils.py index bb1a0c3f9ba..2fcd740841e 100644 --- a/readthedocs/oauth/utils.py +++ b/readthedocs/oauth/utils.py @@ -73,12 +73,11 @@ def update_webhook(project, integration, request=None): project.has_valid_webhook = True project.save() return True - else: - messages.error( - request, - _('Webhook activation failed. ' - 'Make sure you have the necessary permissions.') - ) - project.has_valid_webhook = False - project.save() - return False + messages.error( + request, + _('Webhook activation failed. ' + 'Make sure you have the necessary permissions.') + ) + project.has_valid_webhook = False + project.save() + return False diff --git a/readthedocs/privacy/backend.py b/readthedocs/privacy/backend.py index 6f6cbe71c3f..64ce5a84447 100644 --- a/readthedocs/privacy/backend.py +++ b/readthedocs/privacy/backend.py @@ -38,29 +38,25 @@ def for_user_and_viewer(self, user, viewer): def for_admin_user(self, user=None): if user.is_authenticated(): return self.filter(users__in=[user]) - else: - return self.none() + return self.none() def public(self, user=None): queryset = self.filter(privacy_level=constants.PUBLIC) if user: return self._add_user_repos(queryset, user) - else: - return queryset + return queryset def protected(self, user=None): queryset = self.filter(privacy_level__in=[constants.PUBLIC, constants.PROTECTED]) if user: return self._add_user_repos(queryset, user) - else: - return queryset + return queryset def private(self, user=None): queryset = self.filter(privacy_level=constants.PRIVATE) if user: return self._add_user_repos(queryset, user) - else: - return queryset + return queryset # Aliases diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 90eef739df8..5c8c2818ae0 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -357,8 +357,7 @@ def get_builds_url(self): def get_canonical_url(self): if getattr(settings, 'DONT_HIT_DB', True): return apiv2.project(self.pk).canonical_url().get()['url'] - else: - return self.get_docs_url() + return self.get_docs_url() def get_subproject_urls(self): """List subproject URLs @@ -371,9 +370,8 @@ def get_subproject_urls(self): apiv2.project(self.pk) .subprojects() .get()['subprojects'])] - else: - return [(proj.child.slug, proj.child.get_docs_url()) - for proj in self.subprojects.all()] + return [(proj.child.slug, proj.child.get_docs_url()) + for proj in self.subprojects.all()] def get_production_media_path(self, type_, version_slug, include_file=True): """ @@ -754,8 +752,7 @@ def get_default_branch(self): """Get the version representing 'latest'""" if self.default_branch: return self.default_branch - else: - return self.vcs_repo().fallback_branch + return self.vcs_repo().fallback_branch def add_subproject(self, child, alias=None): subproject, __ = ProjectRelationship.objects.get_or_create( diff --git a/readthedocs/projects/version_handling.py b/readthedocs/projects/version_handling.py index 293bea6904a..7dd58f49e33 100644 --- a/readthedocs/projects/version_handling.py +++ b/readthedocs/projects/version_handling.py @@ -158,8 +158,7 @@ def highest_version(version_list): versions = sort_versions(version_list) if versions: return versions[0] - else: - return (None, None) + return (None, None) def determine_stable_version(version_list): @@ -177,5 +176,4 @@ def determine_stable_version(version_list): if versions: version_obj, comparable = versions[0] return version_obj - else: - return None + return None diff --git a/readthedocs/projects/views/public.py b/readthedocs/projects/views/public.py index 469547cd91e..bacb93684ca 100644 --- a/readthedocs/projects/views/public.py +++ b/readthedocs/projects/views/public.py @@ -246,8 +246,7 @@ def version_filter_autocomplete(request, project_slug): }, context_instance=RequestContext(request), ) - else: - return HttpResponse(status=400) + return HttpResponse(status=400) def file_autocomplete(request, project_slug): diff --git a/readthedocs/redirects/models.py b/readthedocs/redirects/models.py index b4aacfc7e56..687c3ab0323 100644 --- a/readthedocs/redirects/models.py +++ b/readthedocs/redirects/models.py @@ -80,8 +80,7 @@ def __unicode__(self): return ugettext('Page Redirect: %s -> %s' % ( self.from_url, self.to_url)) - else: - return ugettext('Redirect: %s' % self.get_redirect_type_display()) + return ugettext('Redirect: %s' % self.get_redirect_type_display()) def get_full_path(self, filename, language=None, version_slug=None): """ diff --git a/readthedocs/restapi/permissions.py b/readthedocs/restapi/permissions.py index 4f5cc066869..ce9831d0d6c 100644 --- a/readthedocs/restapi/permissions.py +++ b/readthedocs/restapi/permissions.py @@ -17,8 +17,7 @@ class CommentModeratorOrReadOnly(permissions.BasePermission): def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: return True # TODO: Similar logic to #1084 - else: - return AdminPermission.is_admin(request.user, obj.node.project) + return AdminPermission.is_admin(request.user, obj.node.project) class RelatedProjectIsOwner(permissions.BasePermission): diff --git a/readthedocs/restapi/utils.py b/readthedocs/restapi/utils.py index 32d0b7b5134..ed53d18761f 100644 --- a/readthedocs/restapi/utils.py +++ b/readthedocs/restapi/utils.py @@ -74,8 +74,7 @@ def delete_versions(project, version_data): log.info("(Sync Versions) Deleted Versions: [%s]", ' '.join(ret_val)) to_delete_qs.delete() return ret_val - else: - return set() + return set() def index_search_request(version, page_list, commit, project_scale, page_scale, diff --git a/readthedocs/rtd_tests/mocks/mock_api.py b/readthedocs/rtd_tests/mocks/mock_api.py index f7d9dc1fd5f..1651b406025 100644 --- a/readthedocs/rtd_tests/mocks/mock_api.py +++ b/readthedocs/rtd_tests/mocks/mock_api.py @@ -70,8 +70,7 @@ def get(self, **kwargs): project['repo'] = repo if 'slug' in kwargs: return {'objects': [version], 'project': project} - else: - return version + return version return MockVersion diff --git a/readthedocs/search/parse_json.py b/readthedocs/search/parse_json.py index d0f67bee3e7..edb7f1bc0a7 100644 --- a/readthedocs/search/parse_json.py +++ b/readthedocs/search/parse_json.py @@ -127,5 +127,4 @@ def process_file(filename): def recurse_while_none(element): if element.text is None: return recurse_while_none(element.getchildren()[0]) - else: - return element.text + return element.text diff --git a/readthedocs/search/utils.py b/readthedocs/search/utils.py index 67b99a7fabc..2853a18810c 100644 --- a/readthedocs/search/utils.py +++ b/readthedocs/search/utils.py @@ -50,8 +50,7 @@ def process_mkdocs_json(version, build_dir=True): def recurse_while_none(element): if element.text is None: return recurse_while_none(element.getchildren()[0]) - else: - return element.text + return element.text def valid_mkdocs_json(file_path): diff --git a/readthedocs/vcs_support/backends/bzr.py b/readthedocs/vcs_support/backends/bzr.py index b959e777e64..3bfaeb48d57 100644 --- a/readthedocs/vcs_support/backends/bzr.py +++ b/readthedocs/vcs_support/backends/bzr.py @@ -95,5 +95,4 @@ def checkout(self, identifier=None): self.update() if not identifier: return self.up() - else: - return self.run('bzr', 'switch', identifier) + return self.run('bzr', 'switch', identifier) diff --git a/readthedocs/vcs_support/backends/hg.py b/readthedocs/vcs_support/backends/hg.py index 144352b6185..fb0b6b0b055 100644 --- a/readthedocs/vcs_support/backends/hg.py +++ b/readthedocs/vcs_support/backends/hg.py @@ -16,8 +16,7 @@ def update(self): retcode = self.run('hg', 'status')[0] if retcode == 0: return self.pull() - else: - return self.clone() + return self.clone() def pull(self): (pull_retcode, _, _) = self.run('hg', 'pull') @@ -106,6 +105,5 @@ def checkout(self, identifier=None): if retcode == 0: self.run('hg', 'pull') return self.run('hg', 'update', '-C', identifier) - else: - self.clone() - return self.run('hg', 'update', '-C', identifier) + self.clone() + return self.run('hg', 'update', '-C', identifier)