-
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 6 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 |
---|---|---|
|
@@ -29,13 +29,13 @@ | |
# Add any Sphinx extension module names here, as strings. They can be | ||
# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom | ||
# ones. | ||
import importlib.metadata as importlib_metadata | ||
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. Why not just use 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 changes in |
||
import os | ||
|
||
from datetime import date | ||
|
||
from towncrier import __version__ as towncrier_version | ||
|
||
|
||
towncrier_version = importlib_metadata.version("towncrier") | ||
extensions = [] | ||
|
||
# Add any paths that contain templates here, relative to this directory. | ||
|
@@ -55,19 +55,17 @@ | |
project = "Towncrier" | ||
copyright = "{}, Towncrier contributors. Ver {}".format( | ||
_today.year, | ||
towncrier_version.public(), | ||
importlib_metadata.version("towncrier"), | ||
) | ||
author = "Amber Brown" | ||
|
||
# The version info for the project you're documenting, acts as replacement for | ||
# |version| and |release|, also used in various other places throughout the | ||
# built documents. | ||
# The short X.Y version. | ||
version = "{}.{}.{}".format( | ||
towncrier_version.major, towncrier_version.minor, towncrier_version.micro | ||
) | ||
version = ".".join(towncrier_version.split(".")[:3]) | ||
# The full version, including alpha/beta/rc tags. | ||
release = towncrier_version.public() | ||
release = towncrier_version | ||
|
||
# List of patterns, relative to source directory, that match files and | ||
# directories to ignore when looking for source files. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,14 +7,13 @@ | |
|
||
from __future__ import annotations | ||
|
||
import contextlib | ||
import importlib.metadata as importlib_metadata | ||
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,56 +62,50 @@ 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." | ||
) | ||
try: | ||
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 this change needed ? I was thinking that this was sorted out in #502 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. Right, I think it's just a merge artifact. Reverted |
||
version = importlib_metadata.version(package) | ||
except importlib_metadata.PackageNotFoundError: | ||
raise Exception( | ||
f"No __version__ or metadata version info for the '{package}' package." | ||
) | ||
|
||
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 | ||
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 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 commentThe reason will be displayed to describe this comment to others. Learn more. There was an indentation problem with a test, fixed now |
||
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 |
---|---|---|
|
@@ -9,17 +9,24 @@ | |
|
||
from __future__ import annotations | ||
|
||
from importlib.metadata import PackageNotFoundError, version | ||
|
||
import click | ||
|
||
from ._version import __version__ | ||
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. Why not keep the import ? It's just that now 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. done |
||
from .build import _main as _build_cmd | ||
from .check import _main as _check_cmd | ||
from .click_default_group import DefaultGroup | ||
from .create import _main as _create_cmd | ||
|
||
|
||
try: | ||
_version = version("towncrier") | ||
except PackageNotFoundError: # pragma: no cover | ||
_version = "unknown" | ||
|
||
|
||
@click.group(cls=DefaultGroup, default="build", default_if_no_args=True) | ||
@click.version_option(__version__.public()) | ||
@click.version_option(_version) | ||
def cli() -> None: | ||
""" | ||
Towncrier is a utility to produce useful, summarised news files for your project. | ||
|
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 was hoping that we can set the version in single place, like
src/towncrier/_version.py
and the build system can automatically extract the version from there.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.
The changes in
doc
were recycled but now they are no longer relevant. Reverted