-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
♻️ Remove incremental #627
Changes from 9 commits
90656a1
396a84b
accc883
dd20edf
08de244
cf2df80
198ad35
2cf4e76
57022cc
7e9d4f0
03894fe
1f35abd
d96226a
c23eade
2e059fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,14 +7,12 @@ | |
|
||
from __future__ import annotations | ||
|
||
import contextlib | ||
import sys | ||
|
||
from importlib import import_module | ||
from importlib.metadata import version as metadata_version | ||
from types import ModuleType | ||
from typing import Any | ||
|
||
from incremental import Version as IncrementalVersion | ||
|
||
|
||
if sys.version_info >= (3, 10): | ||
|
@@ -63,17 +61,15 @@ def get_version(package_dir: str, package: str) -> str: | |
Try to extract the version from the distribution version metadata that matches | ||
`package`, then fall back to looking for the package in `package_dir`. | ||
""" | ||
version: Any | ||
version: str | ||
|
||
# First try to get the version from the package metadata. | ||
if version := _get_metadata_version(package): | ||
return version | ||
|
||
# When no version if found, fall back to looking for the package in `package_dir`. | ||
module = _get_package(package_dir, package) | ||
|
||
version = getattr(module, "__version__", None) | ||
|
||
if not version: | ||
raise Exception( | ||
f"No __version__ or metadata version info for the '{package}' package." | ||
|
@@ -82,37 +78,30 @@ def get_version(package_dir: str, package: str) -> str: | |
if isinstance(version, str): | ||
return version.strip() | ||
|
||
if isinstance(version, IncrementalVersion): | ||
# FIXME:https://github.com/twisted/incremental/issues/81 | ||
# Incremental uses `.rcN`. | ||
# importlib uses `rcN` (without a dot separation). | ||
# Here we make incremental work like importlib. | ||
return version.base().strip().replace(".rc", "rc") | ||
|
||
if isinstance(version, tuple): | ||
return ".".join(map(str, version)).strip() | ||
|
||
# Try duck-typing as an Incremental version. | ||
if hasattr(version, "base"): | ||
try: | ||
version = str(version.base()).strip() | ||
# Incremental uses `X.Y.rcN`. | ||
# Standardize on importlib (and PEP440) use of `X.YrcN`: | ||
return version.replace(".rc", "rc") # type: ignore | ||
except TypeError: | ||
pass | ||
|
||
raise Exception( | ||
"I only know how to look at a __version__ that is a str, " | ||
"an Increment Version, or a tuple. If you can't provide " | ||
"that, use the --version argument and specify one." | ||
"Version must be a string, tuple, or an Incremental Version." | ||
" If you can't provide that, use the --version argument and specify one." | ||
) | ||
|
||
|
||
def get_project_name(package_dir: str, package: str) -> str: | ||
module = _get_package(package_dir, package) | ||
|
||
version = getattr(module, "__version__", None) | ||
# Incremental has support for package names, try duck-typing it. | ||
with contextlib.suppress(AttributeError): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that we are missing a brach test here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The miss indicator was a bit confusing for me, should be gone now hopefully |
||
return str(version.package) # type: ignore | ||
|
||
if not version: | ||
# welp idk | ||
return package.title() | ||
|
||
if isinstance(version, str): | ||
return package.title() | ||
|
||
if isinstance(version, IncrementalVersion): | ||
# Incremental has support for package names | ||
return version.package | ||
|
||
raise TypeError(f"Unsupported type for __version__: {type(version)}") | ||
return package.title() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Remove incremental dependency from towncrier. | ||
|
||
Towncrier can still read incremental versions, it just doesn't rely on the package itself any more. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,26 @@ | ||
# Copyright (c) Amber Brown, 2015 | ||
# See LICENSE for details. | ||
|
||
from incremental import Version | ||
from twisted.trial.unittest import TestCase | ||
|
||
from towncrier._version import _hatchling_version | ||
import towncrier | ||
|
||
|
||
class TestPackaging(TestCase): | ||
def test_version_warning(self): | ||
def test_version_attr(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure why this change is needed. The previous test look good enough to me. :) I am misssing something ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous test was checking for the presence of an Incremental Version object (which obviously no longer exists in towncrier) I've replaced it with a test to assert that a |
||
""" | ||
Import __version__ from towncrier returns an Incremental version object | ||
and raises a warning. | ||
towncrier.__version__ was deprecated, but still exists for now. | ||
""" | ||
with self.assertWarnsRegex( | ||
DeprecationWarning, "Accessing towncrier.__version__ is deprecated.*" | ||
): | ||
from towncrier import __version__ | ||
|
||
self.assertIsInstance(__version__, Version) | ||
self.assertEqual(_hatchling_version, __version__.short()) | ||
def access__version(): | ||
return towncrier.__version__ | ||
|
||
expected_warning = ( | ||
"Accessing towncrier.__version__ is deprecated and will be " | ||
"removed in a future release. Use importlib.metadata directly " | ||
"to query for towncrier's packaging metadata." | ||
) | ||
|
||
self.assertWarns( | ||
DeprecationWarning, expected_warning, __file__, access__version | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why coverage is reporting this line as not covered.... we need to check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was an indentation problem with a test, fixed now