Remove incremental #491

SmileyChris · 2023-03-27T04:52:30Z

Description

Remove Incremental as a requirement (but still support its use for project versions/names)

Fixes #308 and #398

Checklist

Make sure changes are covered by existing or new tests.
For at least one Python version, make sure local test run is green.
Create a file in src/towncrier/newsfragments/. Describe your
change and include important information. Your change will be included in the public release notes.
Make sure all GitHub Actions checks are green (they are automatically checking all of the above).
Ensure docs/tutorial.rst is still up-to-date.
If you add new CLI arguments (or change the meaning of existing ones), make sure docs/cli.rst reflects those changes.
If you add new configuration options (or change the meaning of existing ones), make sure docs/configuration.rst reflects those changes.

codecov · 2023-03-27T04:57:18Z

Codecov Report

Patch coverage: 86.04% and project coverage change: -0.80 ⚠️

Comparison is base (70da86f) 99.84% compared to head (8667326) 99.05%.

❗ Current head 8667326 differs from pull request most recent head aa5e214. Consider uploading reports for the commit aa5e214 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk     #491      +/-   ##
==========================================
- Coverage   99.84%   99.05%   -0.80%     
==========================================
  Files          13       12       -1     
  Lines         631      632       +1     
  Branches      146      143       -3     
==========================================
- Hits          630      626       -4     
- Misses          0        4       +4     
- Partials        1        2       +1

Impacted Files	Coverage Δ
src/towncrier/__init__.py	`100.00% <ø> (ø)`
src/towncrier/_shell.py	`89.47% <66.66%> (-10.53%)`	⬇️
src/towncrier/_settings/load.py	`98.01% <84.61%> (-1.99%)`	⬇️
src/towncrier/_project.py	`95.74% <88.88%> (-4.26%)`	⬇️
src/towncrier/build.py	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/towncrier/test/test_project.py

adiroiban · 2023-06-05T09:55:23Z

Thanks, Chris for working on this.

I am working to make a new release of towncrier, and the plan is to drop 3.7 support.

So this PR should be even simpler.

I am happy to drop the incremental dependency from towncrier.
I think that managing it via a simple string is good enough.
The version should only be modified by the release manager and the release manager should be familiar with the versioning scheme.

I think that using Version('towncrier', 19, 9, 0, release_candidate=1) instead of just 19.9.0.rc1 is overengineering

SmileyChris · 2023-06-05T23:07:04Z

@adiroiban cool! I'd say merge this first then work on dropping the 3.7 support, otherwise I can update this after you've changed the automated tests so this doesn't fail when I remove the import try/excepts.

(also a review of #482 would be nice, it would be a good addition before the new release ;) )

adiroiban

The changes look good.

Only minor comments.

I think that it would be nice to be able to still do from towncrier._version import version or something similar, without having to worry about importlib each time.

Now, I don't know if it should raise an exception or return a dummy string, when the version was not found... I would go with a "dummy/placeholder" string.

adiroiban · 2023-06-07T13:33:37Z

src/towncrier/_shell.py

 from .build import _main as _build_cmd
 from .check import _main as _check_cmd
 from .create import _main as _create_cmd


+try:


Maybe we can still keep the _version.py file but this time just just a _version string variable.

I this way, we don't have to repeat the importlib code each time we want to fetch the version

We can do this lazily using towncrier.__version__ if the block that I've commented on earlier is ported and kill two birds with one stone.

adiroiban · 2023-06-07T13:33:57Z

src/towncrier/_shell.py

+ from importlib_metadata import PackageNotFoundError, version # type: ignore
+
+
+try:


this code can be included in the updated _version.py code

adiroiban · 2023-06-07T13:35:04Z

src/towncrier/test/test_packaging.py

 """
- Import __version__ from towncrier returns an Incremental version object
- and raises a warning.
+ towncrier.__version__ was deprecated, now no longer exists.


not sure if towncrier.__version__ was considered public or private API.

For private API, raising AttributeError is ok.
If it's public API, we need to raise a separate error with an info message.

src/towncrier/test/test_project.py

adiroiban · 2023-06-07T13:42:12Z

@SmileyChris #482 it's on my todo list... but we can trigger a release anytime.

hynek · 2023-06-12T14:09:55Z

JFTR: if you'd like to entertain the idea of using hatch-vcs to deduplicate version information, I'm happy to help. I used to be skeptical to the idea, but now I've moved all my projects to it. If configured correctly, it also allows for continuous uploads to Test PyPI: https://test.pypi.org/project/structlog/#history

I guess the "you" here is mostly towards @adiroiban since he's doing all the release work and @glyph because he's The Twisted King.

adiroiban · 2023-06-12T14:27:10Z

To me hatch-vcs is over-engineering.
I am not sure what problem it tries to solve.

Is it that hard to keep the version as a text inside a .toml file? :)

hynek · 2023-06-12T14:32:31Z

Well, it's not. :) But it deduplicates the version information to one place (we do git tags too) and enables the continuous uploads I've mentioned. But obviously, if you don't want it, it's not happening. Just wanted to offer. :)

hynek · 2023-06-23T09:41:53Z

this needs a rebase / conflict resolution

SmileyChris · 2023-06-23T12:19:54Z

Removed the python 3.7 related fallbacks, updated the version in the toml, and rebased against trunk.
(and pushing again with no change because the checknewsfragment test failed for no good reason 😖 )

hynek

I've added a few notes, but ultimately I'm unconditionally yielding the decisions to @adiroiban since this has been his dream for years. ;)

hynek · 2023-07-09T11:39:27Z

src/towncrier/__init__.py

-def __getattr__(name: str) -> Version:
- if name != "__version__":
- raise AttributeError(f"module {__name__} has no attribute {name}")
-
- import warnings
-
- from ._version import __version__
-
- warnings.warn(
- "Accessing towncrier.__version__ is deprecated and will be "
- "removed in a future release. Use importlib.metadata directly "
- "to query for towncrier's packaging metadata.",
- DeprecationWarning,
- stacklevel=2,
- )
-
- return __version__


I'm not sure this should be removed. It seems like there's still a bit of software relying on pkg.__version__ such that I've starte de-deprecating it in my own packages. See also python-attrs/attrs#1136

(to be clear: it has to be ported, not just left around ;))

hynek · 2023-07-09T11:40:10Z

src/towncrier/_project.py

@@ -38,47 +38,45 @@ def _get_package(package_dir: str, package: str) -> ModuleType:


 def get_version(package_dir: str, package: str) -> str:
- module = _get_package(package_dir, package)
+ version: Any


Why Any? Any opts version completely out of type checking, is there a rescue for that?

hynek · 2023-07-09T11:41:32Z

src/towncrier/_project.py

+ try:
+ return str(version.package) # type: ignore
+ except AttributeError:
+ pass


Suggested change

try:

return str(version.package) # type: ignore

except AttributeError:

pass

with contextlib.suppress(AttributeError):

return str(version.package) # type: ignore

is more idiomatic, I think

hynek · 2023-07-09T11:43:20Z

src/towncrier/_shell.py

 from .build import _main as _build_cmd
 from .check import _main as _check_cmd
 from .create import _main as _create_cmd


+try:


We can do this lazily using towncrier.__version__ if the block that I've commented on earlier is ported and kill two birds with one stone.

hynek · 2023-07-09T11:43:51Z

src/towncrier/test/test_packaging.py



 class TestPackaging(TestCase):
- def test_version_warning(self):
+ def no_version_attr(self):


I don't think this runs if it doesn't have a test_ prefix.

adiroiban · 2023-07-09T11:44:38Z

src/towncrier/_project.py

 if not version:
- raise Exception("No __version__, I don't know how else to look")
+ try:


it looks like this is not only about removing incremental, but also about using importlib_metadata to get the version of the target project

I think is best to do this as part of a separate PR ..l like #502

SmileyChris force-pushed the remove-incremental branch 2 times, most recently from 27a8f71 to cc040d0 Compare May 2, 2023 02:03

SmileyChris marked this pull request as ready for review May 2, 2023 03:37

SmileyChris requested a review from a team as a code owner May 2, 2023 03:37

chrysle reviewed May 2, 2023

View reviewed changes

src/towncrier/test/test_project.py Show resolved Hide resolved

This was referenced May 3, 2023

#432 Use importlib.metadata.version to get the version #502

Merged

traceback on missing configuration #441

Open

SmileyChris force-pushed the remove-incremental branch from d1f829a to ad93b60 Compare May 11, 2023 03:52

adiroiban requested changes Jun 7, 2023

View reviewed changes

adiroiban mentioned this pull request Jun 12, 2023

Move the release GHA job into a separate workflow #519

Open

SmileyChris force-pushed the remove-incremental branch 2 times, most recently from 26c694c to 7c7a930 Compare June 23, 2023 12:18

SmileyChris force-pushed the remove-incremental branch from 7c7a930 to e0df0a7 Compare June 23, 2023 12:22

SmileyChris added 2 commits June 24, 2023 00:28

Remove incremental

90656a1

Add news fragment

396a84b

SmileyChris force-pushed the remove-incremental branch from e0df0a7 to 396a84b Compare June 23, 2023 12:29

SmileyChris and others added 2 commits June 24, 2023 01:16

Update tutorial for accuracy around detecting versions

accc883

Merge branch 'trunk' into remove-incremental

aa5e214

hynek requested changes Jul 9, 2023

View reviewed changes

adiroiban reviewed Jul 9, 2023

View reviewed changes

adiroiban mentioned this pull request Nov 7, 2023

Remove own usage of incremental. #398

Closed

4 tasks

adiroiban added the needs-changes label Nov 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove incremental #491

Remove incremental #491

SmileyChris commented Mar 27, 2023 •

edited

codecov bot commented Mar 27, 2023 •

edited

adiroiban commented Jun 5, 2023

SmileyChris commented Jun 5, 2023

adiroiban left a comment

adiroiban Jun 7, 2023

hynek Jul 9, 2023

adiroiban Jun 7, 2023

adiroiban Jun 7, 2023

adiroiban commented Jun 7, 2023

hynek commented Jun 12, 2023

adiroiban commented Jun 12, 2023

hynek commented Jun 12, 2023

hynek commented Jun 23, 2023

SmileyChris commented Jun 23, 2023 •

edited

hynek left a comment

hynek Jul 9, 2023

hynek Jul 9, 2023

hynek Jul 9, 2023

hynek Jul 9, 2023

hynek Jul 9, 2023

adiroiban Jul 9, 2023 •

edited

		from importlib_metadata import PackageNotFoundError, version # type: ignore


		try:

Remove incremental #491

Are you sure you want to change the base?

Remove incremental #491

Conversation

SmileyChris commented Mar 27, 2023 • edited

Description

Checklist

codecov bot commented Mar 27, 2023 • edited

Codecov Report

adiroiban commented Jun 5, 2023

SmileyChris commented Jun 5, 2023

adiroiban left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban commented Jun 7, 2023

hynek commented Jun 12, 2023

adiroiban commented Jun 12, 2023

hynek commented Jun 12, 2023

hynek commented Jun 23, 2023

SmileyChris commented Jun 23, 2023 • edited

hynek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban Jul 9, 2023 • edited

Choose a reason for hiding this comment

SmileyChris commented Mar 27, 2023 •

edited

codecov bot commented Mar 27, 2023 •

edited

SmileyChris commented Jun 23, 2023 •

edited

adiroiban Jul 9, 2023 •

edited