Add Pydocstyle

.pydocstyle

@@ -0,0 +1,2 @@
+[pydocstyle]
+convention = google

.vscode/settings.json

@@ -9,5 +9,6 @@
   "python.testing.pytestEnabled": false,
   "python.testing.unittestEnabled": true,
   "python.linting.flake8Enabled": true,
-  "python.linting.enabled": true
+  "python.linting.enabled": true,
+  "python.linting.pydocstyleEnabled": true
 }

BasicDevTests.py

@@ -9,6 +9,7 @@
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
+"""Check that python code in the package aligns with pep8 and file encoding."""
 
 import glob
 import os
@@ -17,7 +18,7 @@
 import re
 
 
-def TestEncodingOk(apath, encodingValue):
+def TestEncodingOk(apath, encodingValue): # noqa
     try:
         with open(apath, "rb") as f_obj:
             f_obj.read().decode(encodingValue)
@@ -28,7 +29,7 @@ def TestEncodingOk(apath, encodingValue):
     return True
 
 
-def TestFilenameLowercase(apath):
+def TestFilenameLowercase(apath): # noqa
     if apath != apath.lower():
         logging.critical(f"Lowercase failure: file {apath} not lower case path")
         logging.error(f"\n\tLOWERCASE: {apath.lower()}\n\tINPUTPATH: {apath}")
@@ -37,8 +38,7 @@ def TestFilenameLowercase(apath):
 
 
 def PackageAndModuleValidCharacters(apath):
-    ''' check pep8 recommendations for package and module names'''
-
+    """Check pep8 recommendations for package and module names."""
     match = re.match('^[a-z0-9_/.]+$', apath.replace("\\", "/"))
     if match is None:
         logging.critical(
@@ -47,14 +47,14 @@ def PackageAndModuleValidCharacters(apath):
     return True
 
 
-def TestNoSpaces(apath):
+def TestNoSpaces(apath): # noqa
     if " " in apath:
         logging.critical(f"NoSpaces failure: file {apath} has spaces in path")
         return False
     return True
 
 
-def TestRequiredLicense(apath):
+def TestRequiredLicense(apath): # noqa
     licenses = ["SPDX-License-Identifier: BSD-2-Clause-Patent", ]
     try:
         with open(apath, "rb") as f_obj:

ConfirmVersionAndTag.py

@@ -6,7 +6,7 @@
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
-
+"""Script to check that the wheel/package created is aligned on a git tag."""
 import glob
 import os
 import sys

azure-pipelines/templates/build-test-job.yml

@@ -33,6 +33,8 @@ jobs:
 
   - template: flake8-test-steps.yml
 
+  - template: pydocstyle-test-steps.yml
+
   - template: spell-test-steps.yml
 
   - template: markdown-lint-steps.yml

azure-pipelines/templates/pydocstyle-test-steps.yml

@@ -0,0 +1,27 @@
+# File pydocstyle-test-steps.yml
+#
+# template file to run pydocstyle and if error publish log
+#
+# Copyright (c) Microsoft Corporation
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+parameters:
+  none: ''
+
+steps:
+- script: pydocstyle .
+  displayName: 'Run pydocstyle'
+  condition: succeededOrFailed()
+
+# Only capture and archive the lint log on failures.
+- script: pydocstyle . > pydocstyle.err.log
+  displayName: 'Capture pydocstyle failures'
+  condition: Failed()
+
+- task: PublishBuildArtifacts@1
+  inputs:
+    pathtoPublish: 'pydocstyle.err.log' 
+    artifactName: 'Pydocstyle Error log file'
+  continueOnError: true
+  condition: Failed()

docs/developing.md

@@ -94,13 +94,19 @@ out all the different parts.
     (whitespace, indentation, etc). In VSCode open the py file and use
     ++alt+shift+f++ to auto format.
 
-2. Run the `BasicDevTests.py` script to check file encoding, file naming, etc
+2. Run a Basic Python docstring Check (using pydocstring) and resolve any issues
+
+    ``` cmd
+    pydocstyle .
+    ```
+
+3. Run the `BasicDevTests.py` script to check file encoding, file naming, etc
 
     ```cmd
     BasicDevTests.py
     ```
 
-3. Run pytest with coverage data collected
+4. Run pytest with coverage data collected
 
     ``` cmd
     pytest -v --junitxml=test.junit.xml --html=pytest_report.html --self-contained-html --cov=edk2toolext --cov-report html:cov_html --cov-report xml:cov.xml --cov-config .coveragerc
@@ -112,17 +118,17 @@ out all the different parts.
     Coverage is uploaded to `codecov.io`. For more information, review
     `coverage.md` in the docs folder.
 
-4. Look at the reports
+5. Look at the reports
     * pytest_report.html
     * cov_html/index.html
 
-5. Run the spell checker
+6. Run the spell checker
 
     ```cmd
     cspell -c .cspell.json "**/*.py" "**/*.md"
     ```
 
-6. Run the markdown linter
+7. Run the markdown linter
 
     ```cmd
     markdownlint "**/*.md"

edk2toolext/__init__.py

@@ -3,3 +3,5 @@
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
+
+# noqa

edk2toolext/base_abstract_invocable.py

@@ -5,6 +5,8 @@
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
+
+"""The Base abstract Invocable that all other invocables should inherit from."""
 import os
 import sys
 import logging
@@ -16,69 +18,151 @@
 
 
 class BaseAbstractInvocable(object):
+    """The Abstract Invocable.
+
+    The base abstract invocable that other invocables should inherit from.
+    Provides base functionality to configure logging and the environment.
 
+    Attributes:
+        log_filename (str): logfile path
+        plugin_manager (PluginManager): the plugin manager
+        helper (HelperFunctions): container for all helper functions
+    """
     def __init__(self):
+        """Init the Invocable."""
         self.log_filename = None
         return
 
     def ParseCommandLineOptions(self):
-        ''' parse arguments '''
+        """Parse command line arguments.
+
+        TIP: Required Override in a subclass
+
+        HINT: argparse.ArgumentParser
+        """
         raise NotImplementedError()
 
     def GetWorkspaceRoot(self):
-        ''' Return the workspace root for initializing the SDE '''
+        """Return the workspace root for initializing the SDE.
+
+        TIP: Required Override in a subclass
+
+        The absolute path to the root of the workspace
+
+        Returns:
+            (str): path to workspace root
+
+        """
         raise NotImplementedError()
 
     def GetActiveScopes(self):
-        ''' return tuple containing scopes that should be active for this process '''
+        """Return tuple containing scopes that should be active for this process.
+
+        TIP: Required Override in a subclass
+
+        TIP: A single scope should end in a comma i.e. (scope,)
+
+        Returns:
+            (Tuple): scopes
+        """
         raise NotImplementedError()
 
     def GetSkippedDirectories(self):
-        ''' Return tuple containing workspace-relative directory paths that should be skipped for processing.
-        Absolute paths are not supported. '''
+        """Return tuple containing workspace-relative directory paths that should be skipped for processing.
+
+        TIP: Optional Override in a subclass
+
+        WARNING: Absolute paths are not supported.
+
+        TIP: A single directory should end with a comma i.e. (dir,)
+
+        Returns:
+            (Tuple): directories
+        """
         return ()
 
     def GetLoggingLevel(self, loggerType):
-        ''' Get the logging level for a given type (return Logging.Level)
+        """Get the logging level depending on logger type.
+
+        TIP: Required Override in a subclass
+
+        Returns:
+            (Logging.Level): The logging level
+
+        HINT: loggerType possible values
         base == lowest logging level supported
         con  == Screen logging
         txt  == plain text file logging
         md   == markdown file logging
-        '''
+        HINT: Return None for no logging for this type.
+        """
         raise NotImplementedError()
 
     def GetLoggingFolderRelativeToRoot(self):
-        ''' Return a path to folder for log files '''
+        """Return the path to a directory to hold all log files.
+
+        TIP: Required Override in a subclass
+
+        Returns:
+            (str): path to the directory
+        """
         raise NotImplementedError()
 
     def InputParametersConfiguredCallback(self):
-        ''' This function is called once all the input parameters
-        are collected and can be used to initialize environment
-        '''
+        """A Callback once all input parameters are collected.
+
+        TIP: Optional override in subclass
+        If you need to do something after input variables have been configured.
+        """
         pass
 
     def GetVerifyCheckRequired(self):
-        ''' Will call self_describing_environment.VerifyEnvironment if this returns True '''
+        """Will call self_describing_environment.VerifyEnvironment if this returns True.
+
+        TIP: Optional override in a subclass
+
+        Returns:
+            (bool): whether verify check is required or not
+        """
         return True
 
     def GetLoggingFileName(self, loggerType):
-        ''' Get the logging file name for the type.
-        Return None if the logger shouldn't be created
+        """Get the logging File name.
 
-        base == lowest logging level supported
-        con  == Screen logging
-        txt  == plain text file logging
-        md   == markdown file logging
-        '''
+        TIP: Required Override this in a subclass
+        Provides logger file name customization.
+
+        Args:
+            loggerType: values can be base, con, txt, md. See hint below
+
+        Returns:
+            (str): filename
+
+        HINT: Return None if the logger shouldn't be created
+
+        HINT: loggerType possible values
+            base == lowest logging level supported
+            con  == Screen logging
+            txt  == plain text file logging
+            md   == markdown file logging
+        """
         raise NotImplementedError()
 
     def Go(self):
-        ''' Main function to run '''
+        """Main function to run.
+
+        Main function to run after the environment and logging has been configured.
+
+        TIP: Required Override in a subclass
+        """
         raise NotImplementedError()
 
     def ConfigureLogging(self):
-        ''' Set up the logging.  This function only needs to be overridden if new behavior is needed'''
+        """Sets up the logging.
 
+        TIP: Optional override in a subclass
+        Only if new behavior is needed.
+        """
         logger = logging.getLogger('')
         logger.setLevel(self.GetLoggingLevel("base"))
 
@@ -107,8 +191,12 @@ def ConfigureLogging(self):
         return
 
     def Invoke(self):
-        ''' Main process function.  Should not need to be overwritten '''
+        """Main process function.
+
+        What actually configure logging and the environment.
 
+        WARNING: Do not override this method
+        """
         self.ParseCommandLineOptions()
         self.ConfigureLogging()
         self.InputParametersConfiguredCallback()

edk2toolext/bin/__init__.py

@@ -3,3 +3,5 @@
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
+
+# noqa

edk2toolext/bin/nuget.py

@@ -5,6 +5,7 @@
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
+"""This module contains code that knows how to download nuget."""
 import os
 import urllib.error
 import urllib.request
@@ -17,11 +18,18 @@
 
 
 def DownloadNuget(unpack_folder: str = None) -> list:
-    '''
-     Downloads a version of NuGet to the specific folder as NuGet.exe
-     If the file already exists, it won't be redownloaded.
-     The file will be checked against a SHA256 hash for accuracy
-    '''
+    """Downloads a version of NuGet to the specific folder as Nuget.exe.
+
+    If the file already exists, it won't be redownloaded.
+    The file will be checked against a SHA256 hash for accuracy
+
+    Args:
+        unpack_folder (str): where to download NuGet
+
+    Raises:
+        (HTTPError): Issue downloading NuGet
+        (RuntimeError): Sha256 did not match
+    """
     if unpack_folder is None:
         unpack_folder = os.path.dirname(__file__)