diff --git a/docs/en_us/developers/source/testing/code-quality.rst b/docs/en_us/developers/source/testing/code-quality.rst index d1b4a044ab7ac9db8286529e91f0afd85c691cfe..bbcb180a1b0fda922e274cd0adb988be228d75f7 100644 --- a/docs/en_us/developers/source/testing/code-quality.rst +++ b/docs/en_us/developers/source/testing/code-quality.rst @@ -18,10 +18,7 @@ in the process of introducing other changes. To check the quality of your pull request, just go to the top level of the edx-platform codebase and run:: - $ paver run_quality - -Note: The rake commands ``rake quality``, ``rake pep8``, and ``rake pylint`` are now deprecated -to ``paver run_quality``, ``paver run_pep8``, and ``paver run_pylint`` respectively. + $ rake quality This will print a report of the quality violations that your branch has made. diff --git a/pavelib/__init__.py b/pavelib/__init__.py index 1f1af5c3af9053f518163b31a851ef28da0fafae..f70eb31bd22fd67760d3024256748e2233ce82f9 100644 --- a/pavelib/__init__.py +++ b/pavelib/__init__.py @@ -1,5 +1,5 @@ """ paver commands """ -__all__ = ["assets", "servers", "docs", "prereqs", "quality"] -from . import assets, servers, docs, prereqs, quality +__all__ = ["assets", "servers", "docs", "prereqs"] +from . import assets, servers, docs, prereqs diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py index f4ab708e8aa2d2bd78c19dcfa531545ae8e7514d..753233f6b2bbebfcf485e0204481bb1026ca38ca 100644 --- a/pavelib/prereqs.py +++ b/pavelib/prereqs.py @@ -104,7 +104,6 @@ def install_node_prereqs(): sh('npm install') -@task def install_python_prereqs(): """ Installs Python prerequisites diff --git a/pavelib/quality.py b/pavelib/quality.py deleted file mode 100644 index 3850039d4c2ad8f2e5324bf5e86ddaa9272d1fff..0000000000000000000000000000000000000000 --- a/pavelib/quality.py +++ /dev/null @@ -1,153 +0,0 @@ -""" -Check code quality using pep8, pylint, and diff_quality. -""" -from paver.easy import sh, task, cmdopts, needs -import os -import errno -from .utils.envs import Env - - -def get_or_make_dir(directory_path): - """ - Ensure that a directory exists, and return its path - """ - try: - os.makedirs(directory_path) - except OSError as err: - if err.errno != errno.EEXIST: - # If we get an error other than one that says - # that the file already exists - raise - - return directory_path - - -@task -@needs('pavelib.prereqs.install_python_prereqs') -@cmdopts([ - ("system=", "s", "System to act on"), - ("errors", "e", "Check for errors only"), -]) -def run_pylint(options): - """ - Run pylint on system code - """ - system = getattr(options, 'system', 'lms') - errors = getattr(options, 'errors', False) - - # Directory to put the pylint report in. - # This makes the folder if it doesn't already exist. - report_dir = get_or_make_dir(os.path.join(Env.REPORT_DIR, system)) - - flags = '-E' if errors else '' - - apps = [system] - - for directory in ['djangoapps', 'lib']: - dirs = os.listdir(os.path.join(system, directory)) - apps.extend([d for d in dirs if os.path.isdir(os.path.join(system, directory, d))]) - - apps_list = ' '.join(apps) - - pythonpath_prefix = ( - "PYTHONPATH={system}:{system}/djangoapps:{system}/" - "lib:common/djangoapps:common/lib".format( - system=system - ) - ) - - sh( - "{pythonpath_prefix} pylint {flags} -f parseable {apps} | " - "tee {report_dir}/pylint.report".format( - pythonpath_prefix=pythonpath_prefix, - flags=flags, - apps=apps_list, - report_dir=report_dir - ) - ) - - -@task -@needs('pavelib.prereqs.install_python_prereqs') -@cmdopts([ - ("system=", "s", "System to act on"), -]) -def run_pep8(options): - """ - Run pep8 on system code - """ - system = getattr(options, 'system', 'lms') - - # Directory to put the pep8 report in. - # This makes the folder if it doesn't already exist. - report_dir = get_or_make_dir(os.path.join(Env.REPORT_DIR, system)) - - sh('pep8 {system} | tee {report_dir}/pep8.report'.format(system=system, report_dir=report_dir)) - - -@task -@needs('pavelib.prereqs.install_python_prereqs') -def run_quality(): - """ - Build the html diff quality reports, and print the reports to the console. - """ - - # Directory to put the diff reports in. - # This makes the folder if it doesn't already exist. - dquality_dir = get_or_make_dir(os.path.join(Env.REPORT_DIR, "diff_quality")) - - # Generage diff-quality html report for pep8, and print to console - # If pep8 reports exist, use those - # Otherwise, `diff-quality` will call pep8 itself - - pep8_files = [] - for subdir, _dirs, files in os.walk(os.path.join(Env.REPORT_DIR)): - for f in files: - if f == "pep8.report": - pep8_files.append(os.path.join(subdir, f)) - - pep8_reports = u' '.join(pep8_files) - - sh( - "diff-quality --violations=pep8 --html-report {dquality_dir}/" - "diff_quality_pep8.html {pep8_reports}".format( - dquality_dir=dquality_dir, pep8_reports=pep8_reports) - ) - - sh( - "diff-quality --violations=pep8 {pep8_reports}".format( - pep8_reports=pep8_reports) - ) - - # Generage diff-quality html report for pylint, and print to console - # If pylint reports exist, use those - # Otherwise, `diff-quality` will call pylint itself - - pylint_files = [] - for subdir, _dirs, files in os.walk(os.path.join(Env.REPORT_DIR)): - for f in files: - if f == "pylint.report": - pylint_files.append(os.path.join(subdir, f)) - - pylint_reports = u' '.join(pylint_files) - - pythonpath_prefix = ( - "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:" - "common:common/djangoapps:common/lib" - ) - - sh( - "{pythonpath_prefix} diff-quality --violations=pylint --html-report " - "{dquality_dir}/diff_quality_pylint.html {pylint_reports}".format( - pythonpath_prefix=pythonpath_prefix, - dquality_dir=dquality_dir, - pylint_reports=pylint_reports - ) - ) - - sh( - "{pythonpath_prefix} diff-quality --violations=pylint {pylint_reports}".format( - pythonpath_prefix=pythonpath_prefix, - pylint_reports=pylint_reports - ) - ) diff --git a/pavelib/utils/envs.py b/pavelib/utils/envs.py index 2e4b96e5cd6035b9d74d68dc5ca5cda41223dd3d..0f6930750308e1112725bec7286b623899b14e18 100644 --- a/pavelib/utils/envs.py +++ b/pavelib/utils/envs.py @@ -17,9 +17,6 @@ class Env(object): # Root of the git repository (edx-platform) REPO_ROOT = path(__file__).abspath().parent.parent.parent - # Reports Directory - REPORT_DIR = REPO_ROOT / 'reports' - # Service variant (lms, cms, etc.) configured with an environment variable # We use this to determine which envs.json file to load. SERVICE_VARIANT = os.environ.get('SERVICE_VARIANT', None) diff --git a/rakelib/quality.rake b/rakelib/quality.rake new file mode 100644 index 0000000000000000000000000000000000000000..2624e5e7c23848a7d563caf4acca94b8999f25d7 --- /dev/null +++ b/rakelib/quality.rake @@ -0,0 +1,72 @@ +def run_pylint(system, report_dir, flags='') + apps = Dir["#{system}", "#{system}/djangoapps/*"] + if system != 'lms' + apps += Dir["#{system}/lib/*"] + end + + apps = apps.map do |app| + File.basename(app) + end.select do |app| + app !=~ /.pyc$/ + end.map do |app| + if app =~ /.py$/ + app.gsub('.py', '') + else + app + end + end + + pythonpath_prefix = "PYTHONPATH=#{system}:#{system}/djangoapps:#{system}/lib:common/djangoapps:common/lib" + sh("#{pythonpath_prefix} pylint #{flags} -f parseable #{apps.join(' ')} | tee #{report_dir}/pylint.report") +end + + +[:lms, :cms, :common].each do |system| + report_dir = report_dir_path(system) + directory report_dir + + namespace :pylint do + namespace system do + desc "Run pylint checking for #{system} checking for errors only, and aborting if there are any" + task :errors do + run_pylint(system, report_dir, '-E') + end + end + + desc "Run pylint on all #{system} code" + task system => [report_dir, :install_python_prereqs] do + run_pylint(system, report_dir) + end + end + task :pylint => :"pylint:#{system}" + + namespace :pep8 do + desc "Run pep8 on all #{system} code" + task system => [report_dir, :install_python_prereqs] do + sh("pep8 #{system} | tee #{report_dir}/pep8.report") + end + end + task :pep8 => :"pep8:#{system}" +end + +dquality_dir = File.join(REPORT_DIR, "diff_quality") +directory dquality_dir + +desc "Build the html diff quality reports, and print the reports to the console." +task :quality => [dquality_dir, :install_python_prereqs] do + + # Generage diff-quality html report for pep8, and print to console + # If pep8 reports exist, use those + # Otherwise, `diff-quality` will call pep8 itself + pep8_reports = FileList[File.join(REPORT_DIR, '**/pep8.report')].join(' ') + sh("diff-quality --violations=pep8 --html-report #{dquality_dir}/diff_quality_pep8.html #{pep8_reports}") + sh("diff-quality --violations=pep8 #{pep8_reports}") + + # Generage diff-quality html report for pylint, and print to console + # If pylint reports exist, use those + # Otherwise, `diff-quality` will call pylint itself + pylint_reports = FileList[File.join(REPORT_DIR, '**/pylint.report')].join(' ') + pythonpath_prefix = "PYTHONPATH=$PYTHONPATH:lms:lms/djangoapps:lms/lib:cms:cms/djangoapps:cms/lib:common:common/djangoapps:common/lib" + sh("#{pythonpath_prefix} diff-quality --violations=pylint --html-report #{dquality_dir}/diff_quality_pylint.html #{pylint_reports}") + sh("#{pythonpath_prefix} diff-quality --violations=pylint #{pylint_reports}") +end diff --git a/rakelib/quality_deprecated.rake b/rakelib/quality_deprecated.rake deleted file mode 100644 index 6f5eead3754a132d57929c752cb57fe7c121d0e9..0000000000000000000000000000000000000000 --- a/rakelib/quality_deprecated.rake +++ /dev/null @@ -1,39 +0,0 @@ -# quality tasks deprecated to paver - -require 'colorize' - -def deprecated(deprecated, deprecated_by) - - task deprecated do - - # Need to install paver dependencies for the commands to work! - sh("pip install -r requirements/edx/paver.txt") - - if deprecated.include? "extract" and ARGV.last.downcase == 'extract' - new_cmd = deprecated_by + " --extract" - else - new_cmd = deprecated_by - end - - puts("Task #{deprecated} has been deprecated. Use #{deprecated_by} instead.".red) - sh(new_cmd) - exit - end -end - -deprecated("pep8:cms", "paver run_pep8 --system=cms") -deprecated("pep8:lms", "paver run_pep8 --system=lms") -deprecated("pep8:common", "paver run_pep8 --system=common") -deprecated("pep8", "paver run_pep8") - -deprecated("pylint:cms", "paver run_pylint --system=cms") -deprecated("pylint:lms", "paver run_pylint --system=lms") -deprecated("pylint:common", "paver run_pylint --system=common") -deprecated("pylint", "paver run_pylint") - -deprecated("pylint:cms:errors", "paver run_pylint --system=cms --errors") -deprecated("pylint:lms:errors", "paver run_pylint --system=lms --errors") -deprecated("pylint:common:errors", "paver run_pylint --system=common --errors") -deprecated("pylint:errors", "paver run_pylint --errors") - -deprecated("quality", "paver run_quality")