Closed flyinghyrax closed 4 months ago
Attention: Patch coverage is 85.83333%
with 17 lines
in your changes are missing coverage. Please review.
Project coverage is 83.69%. Comparing base (
4d020e8
) to head (74986a2
). Report is 96 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
src/bandersnatch/configuration.py | 80.00% | 11 Missing and 1 partial :warning: |
src/bandersnatch/main.py | 61.53% | 4 Missing and 1 partial :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
(...finally.)
This should be a much more focused PR for addressing issue #1702 without so much additional refactoring.
I've changed the configuration files embedded in the package like so:
-c
argument is passed file that doesn't exist to create a configuration.[mirror]
configuration options except fordirectory
, with minimal annotation.I changed
BandersnatchConfig
:ConfigParser
. Technically this violates the Liskov substitution principal by giving the subtype a different signature for__init__.
I have mixed feelings about that, but this seemed like a much cleaner way to customize ConfigParser and removed many instances ofBandersnatchConfig().config
or similar.Some notable effects of this include:
ConfigParser.has_option
on the 'mirror' section. Instead, you have to check whether the options value is empty or None..get
on the 'mirror' section will have no effect, because the option will already be present in the ConfigParser mappings.Happy to tweak which options are required/have defaults and dynamic fallback behaviors for things like
root_uri
, if that's something we want to change at this point.Fixes #1702 Fixes #990