On 5/22/19 9:22 AM, Pino Toscano wrote: > On Wednesday, 22 May 2019 15:16:27 CEST Fabiano Fidêncio wrote: >> As suggested by Cole, let's keep just one implementation of >> _parse_iso_date() (the most compatible one) and avoid diverging over >> time. >> >> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx> >> --- >> tests/test_dates.py | 12 ++++-------- >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/tests/test_dates.py b/tests/test_dates.py >> index 49df613..aca7472 100644 >> --- a/tests/test_dates.py >> +++ b/tests/test_dates.py >> @@ -8,14 +8,10 @@ import sys >> from . import util >> >> >> -if sys.version_info >= (3, 7): >> - def _parse_iso_date(date_string): >> - return datetime.date.fromisoformat(date_string) >> -else: >> - def _parse_iso_date(date_string): >> - m = re.match("([0-9]{4})-([0-9]{2})-([0-9]{2})", date_string) >> - assert m >> - return datetime.date(int(m.group(1)), int(m.group(2)), int(m.group(3))) >> +def _parse_iso_date(date_string): >> + m = re.match("([0-9]{4})-([0-9]{2})-([0-9]{2})", date_string) >> + assert m >> + return datetime.date(int(m.group(1)), int(m.group(2)), int(m.group(3))) > > The purposes of having the two implementation are: > - datetime.date.fromisoformat() is available in newer versions, so it > makes sense to leverage it What are the concrete benefits we receive from using the library call, when we still need to maintain the old code path? All the code reuse benefits are pretty much thrown out if we need to maintain the old code path > - my implementation is not optimized as datetime.date.fromisoformat(); > considering this is only a test, it is "good enough" > - having the version check means it is easier to drop the old > implementation when raising the minimum Python version required > Considering we will want to be compatible with running tests on RHEL8 which has python3.6 by default, we are talking a long time before we can depend on python3.7+. In the intervening years, if a dev wants or needs to change this code, we would need to test against two python versions which is a pain, or push it off to CI to catch. It adds complexity for hypothetical benefits. Doesn't seem worth it IMO. But if you don't find that convincing, I won't argue it further :) Thanks, Cole _______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo