From c5766b9b33a3b23189a12aac6c8ca86c4d61b31d Mon Sep 17 00:00:00 2001 From: A_D Date: Mon, 27 Jul 2020 18:55:59 +0200 Subject: [PATCH 01/15] Dont crash when journal_dir is None Ensures that journal_dir is always at least an empty string. Fixes #639 --- monitor.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/monitor.py b/monitor.py index eb63e83c..ac7e8d10 100644 --- a/monitor.py +++ b/monitor.py @@ -118,9 +118,14 @@ class EDLogs(FileSystemEventHandler): def start(self, root): self.root = root - logdir = expanduser(config.get('journaldir') or config.default_journal_dir) # type: ignore # config is weird + journal_dir = config.get('journaldir') or config.default_journal_dir - if not logdir or not isdir(logdir): # type: ignore # config does weird things in its get + if journal_dir is None: + journal_dir = '' + + logdir = expanduser(journal_dir) # type: ignore # config is weird + + if not logdir or not isdir(logdir): self.stop() return False From 1053abaf00fe7c324e217abeae95a618217d093d Mon Sep 17 00:00:00 2001 From: A_D Date: Mon, 27 Jul 2020 19:07:54 +0200 Subject: [PATCH 02/15] Add TODO regarding type config --- monitor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/monitor.py b/monitor.py index ac7e8d10..90988dfd 100644 --- a/monitor.py +++ b/monitor.py @@ -122,7 +122,9 @@ class EDLogs(FileSystemEventHandler): if journal_dir is None: journal_dir = '' - + + # TODO(A_D): this is ignored for type checking due to all the different types config.get returns + # When that is refactored, remove the magic comment logdir = expanduser(journal_dir) # type: ignore # config is weird if not logdir or not isdir(logdir): From 9e18dde834de4ce7eeb8730cc64e87c8df73957c Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 17 Jul 2020 18:27:38 +0100 Subject: [PATCH 03/15] Update PLUGINS.md Adds missing blank line that caused acciental underline/heading --- PLUGINS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/PLUGINS.md b/PLUGINS.md index d6a214b9..9f7bf42a 100644 --- a/PLUGINS.md +++ b/PLUGINS.md @@ -363,6 +363,7 @@ If the player has chosen to "Send flight log and Cmdr status to Inara" this gets called when the player starts the game, enters a new system, docks or undocks. It is called some time after the corresponding `journal_entry()` event. + --- ```python def inara_notify_ship(eventData): From 26a94f247be7d53d7dae61fec8a710dd219860b3 Mon Sep 17 00:00:00 2001 From: A_D Date: Sun, 19 Jul 2020 07:50:44 +0200 Subject: [PATCH 04/15] Add flake8 and autopep8 to requirements.txt --- requirements.txt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/requirements.txt b/requirements.txt index 2327e183..4ef8f1ce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,15 @@ keyring==19.2.0 pathtools>=0.1.2 requests>=2.11.1 watchdog>=0.8.3 + +# Static analysis tools +flake8==3.8.3 +flake8-annotations-coverage==0.0.4 +flake8-cognitive-complexity==0.0.2 +flake8-comprehensions==3.2.3 +flake8-pep3101==1.3.0 +flake8-polyfill==1.0.2 +pep8-naming==0.11.1 + +# Code formatting tools +autopep8==1.5.3 From 536f2ff39319fbba05734160ab316384f913f653 Mon Sep 17 00:00:00 2001 From: A_D Date: Sun, 19 Jul 2020 07:52:06 +0200 Subject: [PATCH 05/15] Add config for autopep8 --- .pyproject.toml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .pyproject.toml diff --git a/.pyproject.toml b/.pyproject.toml new file mode 100644 index 00000000..7ff18c21 --- /dev/null +++ b/.pyproject.toml @@ -0,0 +1,2 @@ +[tool.autopep8] +max_line_length = 120 From 2fa49210d30b8a3121b320f37a88f0b1ba32d9bb Mon Sep 17 00:00:00 2001 From: A_D Date: Sun, 19 Jul 2020 16:53:21 +0200 Subject: [PATCH 06/15] Removed dev deps from requirements.txt --- requirements.txt | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/requirements.txt b/requirements.txt index 4ef8f1ce..5fa59d6a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,17 +1,7 @@ certifi==2019.9.11 keyring==19.2.0 -pathtools>=0.1.2 requests>=2.11.1 watchdog>=0.8.3 - -# Static analysis tools -flake8==3.8.3 -flake8-annotations-coverage==0.0.4 -flake8-cognitive-complexity==0.0.2 -flake8-comprehensions==3.2.3 -flake8-pep3101==1.3.0 -flake8-polyfill==1.0.2 -pep8-naming==0.11.1 - -# Code formatting tools -autopep8==1.5.3 +# argh==0.26.2 watchdog dep +# pyyaml==5.3.1 watchdog dep +semantic-version==2.8.5 From aae4aace6c384b9b95c13ceafcca44d2e0f8477e Mon Sep 17 00:00:00 2001 From: A_D Date: Sun, 19 Jul 2020 16:53:58 +0200 Subject: [PATCH 07/15] Create requirements-dev.txt --- requirements-dev.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 requirements-dev.txt diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 00000000..0666adb3 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,20 @@ +# Static analysis tools +flake8==3.8.3 +flake8-annotations-coverage==0.0.4 +flake8-cognitive-complexity==0.0.2 +flake8-comprehensions==3.2.3 +flake8-pep3101==1.3.0 +flake8-polyfill==1.0.2 +pep8-naming==0.11.1 + +# Code formatting tools +autopep8==1.5.3 + +# HTML changelogs +grip==4.5.2 + +# Packaging +py2exe==0.9.3.2 + +# All of the normal requirements +-r requirements.txt From 7de06fb6eff57caa893e595ff270ed7035b119db Mon Sep 17 00:00:00 2001 From: A_D Date: Sun, 19 Jul 2020 17:42:51 +0200 Subject: [PATCH 08/15] Updated docs Added linting, testing, and new requirements-dev.txt instructions --- Contributing.md | 64 +++++++++++++++++++++++++++++++++++++++++------ docs/Releasing.md | 6 ++--- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/Contributing.md b/Contributing.md index 168c97de..4b7bdaae 100644 --- a/Contributing.md +++ b/Contributing.md @@ -104,7 +104,7 @@ The Semantic Versioning `+` should never be a part of the tag. --- -Work in progress conventions +### Work in progress conventions --- Remember, you should always be working versus a single issue, even if the work is part of a Milestone or Project. There might be cases where issues aren't duplicates, but your work still addresses more than one. In that case @@ -131,6 +131,24 @@ your WIP branch. If there are any non-trivial conflicts when we merge your Pull to rebase your WIP branch on the latest version of the source branch. Otherwise we'll work out how to best merge your changes via comments in the Pull Request. + +#### Linting + +We use flake8 for linting all python source. + +While working on your changes, please ensure that they pass a check from `flake8` using our configuration and plugins. +If you installed `requirements-dev.txt` with pip, you should simply be able to run `flake8 your_files_here` to lint +your files. + +Note that if your PR does not cleanly (or mostly cleanly) pass a linting scan, your PR may be put on hold pending fixes. + +#### Unit testing + +Where possible please write unit tests for your PRs, especially in the case of bug fixes, having regression tests help +ensure that we don't accidentally re-introduce a bug down the line. + +We use the python stdlib library `unittest` for unit testing. + --- General workflow @@ -141,6 +159,8 @@ General workflow 1. In your local copy of *your* fork create an appropriate WIP branch. 1. Develop the changes, testing as you go (no we don't have any actual tests yet). 1. Be as sure as you can that the code works as you intend and hasn't introduced any other bugs or regressions. + 1. Test the codebase as a whole against any unit tests that do exist, and add your own as you can. + 1. Check your code against flake8 periodically. 1. When you're sure the work is final: 1. Push your WIP branch to your fork (you probably should have been doing this as you worked as a form of backup). 1. Access the WIP branch on your fork on GitHub and create a Pull Request. Mention any Issue number(s) that it @@ -162,19 +182,49 @@ Coding Conventions on a separate line, with consistent indentation. Yes: - + + ```python if somethingTrue: - Things we then do + Things_we_then_do() + ``` No: - - if somethingTrue: One thing we do + + ```python + if somethingTrue: One_thing_we_do() + ``` Yes, some existing code still flouts this rule. - + +* **Always** use Line breaks after scope changes. It makes reading code far easier + + Yes: + + ```python + if True: + do_something() + + else: + raise UniverseBrokenException() + + return + ``` + No: + + ```python + if True: + do_something() + else: + raise UniverseBrokenException() + return + ``` + + * Going forwards please do place [type hints](https://docs.python.org/3/library/typing.html) on the declarations of your functions, both their arguments and return types. - + +* In general, please follow [PEP8](https://www.python.org/dev/peps/pep-0008/). + --- Git commit conventions diff --git a/docs/Releasing.md b/docs/Releasing.md index 2f52c67e..ff2ec482 100644 --- a/docs/Releasing.md +++ b/docs/Releasing.md @@ -54,9 +54,9 @@ You will need several pieces of software installed, or the files from their 1. You'll now need to 'pip install' several python modules. 1. Ensure you have `pip` installed. If needs be see [Installing pip](https://pip.pypa.io/en/stable/installing/) - 1. The easiest way is to utilise the `requirements.txt` file: - `pip install -r requirements.txt` - 1. Else check the contents of `requirements.txt` and ensure the modules + 1. The easiest way is to utilise the `requirements-dev.txt` file: + `python -m pip install -r requirements-dev.txt`. This will install all dependencies plus anything required for development + 1. Else check the contents of both `requirements.txt` and `requirements-dev.txt`, and ensure the modules listed there are installed as per the version requirements. If you are using different versions of any of these tools then please ensure From 8f7bd53b5ca8443c228be96b7117c3a54b499db5 Mon Sep 17 00:00:00 2001 From: A_D Date: Sun, 19 Jul 2020 17:47:52 +0200 Subject: [PATCH 09/15] Fixed code blocks --- Contributing.md | 51 +++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/Contributing.md b/Contributing.md index 4b7bdaae..ff683dff 100644 --- a/Contributing.md +++ b/Contributing.md @@ -181,43 +181,44 @@ Coding Conventions * **ALWAYS** place a single-statement control flow body, for control statements such as `if`, `else`, `for`, `foreach`, on a separate line, with consistent indentation. - Yes: +Yes: - ```python - if somethingTrue: - Things_we_then_do() - ``` +```python +if somethingTrue: + Things_we_then_do() +``` - No: +No: - ```python - if somethingTrue: One_thing_we_do() - ``` +```python +if somethingTrue: One_thing_we_do() +``` Yes, some existing code still flouts this rule. * **Always** use Line breaks after scope changes. It makes reading code far easier - Yes: +Yes: - ```python - if True: - do_something() +```python + if True: + do_something() - else: - raise UniverseBrokenException() + else: + raise UniverseBrokenException() - return - ``` - No: + return +``` - ```python - if True: - do_something() - else: - raise UniverseBrokenException() - return - ``` +No: + +```python + if True: + do_something() + else: + raise UniverseBrokenException() + return +``` * Going forwards please do place [type hints](https://docs.python.org/3/library/typing.html) on the declarations of your functions, both their arguments and return From f849f010e27905e0b107766442c3b2fb98ba39fa Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 24 Jul 2020 10:03:56 +0100 Subject: [PATCH 10/15] Releasing.md: Update 'Known Issues' after a stable release. --- docs/Releasing.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/Releasing.md b/docs/Releasing.md index ff2ec482..4c251af8 100644 --- a/docs/Releasing.md +++ b/docs/Releasing.md @@ -306,6 +306,8 @@ updates specifically targets NB: It can take some time for GitHub to show the changed edmarketconnector.xml contents to all users. +**You should now update [Known Issues](https://github.com/EDCD/EDMarketConnector/issues/618) +to reflect anything now fixed in latest release.** Pre-Releases --- From be6a7bcdc5ce11add0f20402eaf88e9a792b4cc1 Mon Sep 17 00:00:00 2001 From: Athanasius Date: Fri, 24 Jul 2020 14:28:34 +0100 Subject: [PATCH 11/15] Releasing.md: Emphasises that sparkle:version is SemVer string. --- docs/Releasing.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/Releasing.md b/docs/Releasing.md index 4c251af8..fcc66daa 100644 --- a/docs/Releasing.md +++ b/docs/Releasing.md @@ -180,7 +180,8 @@ a `stable` release, as well as any social media posts you make. 1. You'll need to change the `` and `<description>` texts to reflect the latest version and the additional changelog. 1. Update the `url` and `sparkle:version` elements of the `<enclosure>` - section. + section. **NB: Yes, sparkle:version should be the Semantic Version + string, not the Windows A.B.C.D form.** 1. As you're working in a version-specific branch, `release-4.0.2`, you can safely commit these changes and push to GitHub. **Do not merge the branch with `releases` until the GitHub release is in From 837ac588cbd6302e370f3397b543f0402570153e Mon Sep 17 00:00:00 2001 From: Athanasius <github@miggy.org> Date: Sun, 26 Jul 2020 20:36:35 +0100 Subject: [PATCH 12/15] Comments out py2exe in requirements-dev.txt as it trips up GitHub Actions docs/Releasing.md has fuller instructions on getting that specific pyexe version installed anyway. --- docs/Releasing.md | 9 ++++++--- requirements-dev.txt | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/Releasing.md b/docs/Releasing.md index fcc66daa..0e704d08 100644 --- a/docs/Releasing.md +++ b/docs/Releasing.md @@ -55,9 +55,12 @@ You will need several pieces of software installed, or the files from their 1. Ensure you have `pip` installed. If needs be see [Installing pip](https://pip.pypa.io/en/stable/installing/) 1. The easiest way is to utilise the `requirements-dev.txt` file: - `python -m pip install -r requirements-dev.txt`. This will install all dependencies plus anything required for development - 1. Else check the contents of both `requirements.txt` and `requirements-dev.txt`, and ensure the modules - listed there are installed as per the version requirements. + `python -m pip install -r requirements-dev.txt`. This will install all + dependencies plus anything required for development *other than py2exe, see + above*. + 1. Else check the contents of both `requirements.txt` and `requirements-dev.txt`, + and ensure the modules listed there are installed as per the version + requirements. If you are using different versions of any of these tools then please ensure that the paths where they're installed match the associated lines in diff --git a/requirements-dev.txt b/requirements-dev.txt index 0666adb3..ab0c4685 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -14,7 +14,9 @@ autopep8==1.5.3 grip==4.5.2 # Packaging -py2exe==0.9.3.2 +# This isn't available via 'pip install', so has to be commented out in order for +# GitHub Action Workflows to not error out +#py2exe==0.9.3.2 # All of the normal requirements -r requirements.txt From 8483b0492300391d739ae33630f72b4ea637befa Mon Sep 17 00:00:00 2001 From: A_D <aunderscored@gmail.com> Date: Tue, 28 Jul 2020 11:24:49 +0200 Subject: [PATCH 13/15] Removed keyring dependency This remove all dependencies on the keyring lib, updates the requirements.txt to reflect that, and ensures that setup.py does not attempt to package it. Any use of the "old" keyring code will now return None and warn about its deprecation. --- config.py | 19 ++++--------------- requirements.txt | 1 - setup.py | 2 -- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/config.py b/config.py index f6ad1acf..b0b887ec 100644 --- a/config.py +++ b/config.py @@ -1,5 +1,6 @@ import numbers import sys +import warnings from os import getenv, makedirs, mkdir, pardir from os.path import expanduser, dirname, exists, isdir, join, normpath from sys import platform @@ -367,25 +368,13 @@ class Config(object): # Common def get_password(self, account): - try: - import keyring - return keyring.get_password(self.identifier, account) - except ImportError: - return None + warnings.warn("password subsystem is no longer supported", DeprecationWarning) def set_password(self, account, password): - try: - import keyring - keyring.set_password(self.identifier, account, password) - except ImportError: - pass + warnings.warn("password subsystem is no longer supported", DeprecationWarning) def delete_password(self, account): - try: - import keyring - keyring.delete_password(self.identifier, account) - except: - pass # don't care - silently fail + warnings.warn("password subsystem is no longer supported", DeprecationWarning) # singleton config = Config() diff --git a/requirements.txt b/requirements.txt index 5fa59d6a..bf29adc9 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,4 @@ certifi==2019.9.11 -keyring==19.2.0 requests>=2.11.1 watchdog>=0.8.3 # argh==0.26.2 watchdog dep diff --git a/setup.py b/setup.py index be40d655..6bf35f8e 100755 --- a/setup.py +++ b/setup.py @@ -73,7 +73,6 @@ if sys.platform=='darwin': 'optimize': 2, 'packages': [ 'requests', - 'keyring.backends', 'sqlite3', # Included for plugins ], 'includes': [ @@ -119,7 +118,6 @@ elif sys.platform=='win32': 'optimize': 2, 'packages': [ 'requests', - 'keyring.backends', 'sqlite3', # Included for plugins ], 'includes': [ From 974872fe9e706542241e19e4cd1cbc401188cae8 Mon Sep 17 00:00:00 2001 From: Athanasius <github@miggy.org> Date: Thu, 30 Jul 2020 14:05:09 +0100 Subject: [PATCH 14/15] PLUGINS.md: Logging is being added, how to prepare * Currently you use `print(...)` * `logging` support is coming, here's how to prepare. --- PLUGINS.md | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/PLUGINS.md b/PLUGINS.md index 9f7bf42a..02daa688 100644 --- a/PLUGINS.md +++ b/PLUGINS.md @@ -55,6 +55,72 @@ import myNotebook as nb ``` For creating UI elements. +--- +### Logging +Currently (still in 4.0.3) the only way to provide any logged output from a +plugin is to use `print(...)` statements. When running the application from +the packaged executeable all output is redirected to a log file. See +[Reporting a problem](https://github.com/EDCD/EDMarketConnector/wiki/Troubleshooting#reporting-a-problem) +for the location of this log file. + +A future version of EDMC will implement proper logging using the Python +`logging` module. Plugin developers should get their code ready for this by +using the following code instead of simple `print(...)' statements + +Insert this at the top-level of your load.py file (so not inside +`plugin_start3()`): +```python +import logging + +from config import appname + +# This could also be returned from plugin_start3() +plugin_name = os.path.basename(os.path.dirname(__file__)) + +# A Logger is used per 'found' plugin to make it easy to include the plugin's +# folder name in the logging output format. +logger = logging.getLogger(f'{appname}.{plugin_name}') + +# If the Logger has handlers then it was already set up by the core code, else +# it needs setting up here. +if not logger.hasHandlers(): + level = logging.INFO # So logger.info(...) is equivalent to print() + + logger.setLevel(level) + logger_channel = logging.StreamHandler() + logger_channel.setLevel(level) + logger_formatter = logging.Formatter(f'%(asctime)s - %(name)s - %(levelname)s - %(module)s:%(lineno)d:%(funcName)s: %(message)s') + logger_formatter.default_time_format = '%Y-%m-%d %H:%M:%S' + logger_formatter.default_msec_format = '%s.%03d' + logger_channel.setFormatter(logger_formatter) + logger.addHandler(logger_channel) +``` + +Then replace `print(...)` statements with one of the following: +```python + logger.info('some info message') # instead of print(...) + + logger.debug('something only for debug') + + logger.warning('Something needs warning about') + + logger.error('Some error happened') + + logger.critical('Something went wrong in a critical manner') + + try: + ... + catch Exception: + # This logs at 'ERROR' level. + # Also automatically includes exception information. + logger.exception('An exception occurred') + + try: + ... + catch Exception as e: + logger.debug('Exception we only note in debug output', exc_info=e) +``` + --- ### Startup From 0e6d4468dae8a6bd6ae2c62ac9e937c4e4446565 Mon Sep 17 00:00:00 2001 From: Athanasius <github@miggy.org> Date: Thu, 30 Jul 2020 14:17:14 +0100 Subject: [PATCH 15/15] PLUGINS.md: Fix typo ' -> ` --- PLUGINS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PLUGINS.md b/PLUGINS.md index 02daa688..d574620b 100644 --- a/PLUGINS.md +++ b/PLUGINS.md @@ -65,7 +65,7 @@ for the location of this log file. A future version of EDMC will implement proper logging using the Python `logging` module. Plugin developers should get their code ready for this by -using the following code instead of simple `print(...)' statements +using the following code instead of simple `print(...)` statements Insert this at the top-level of your load.py file (so not inside `plugin_start3()`):