Repository: twisted/towncrier
Pull request #548 (2023-09-26 10:13:30)

Config location separate from directory containing news file and fragments

# Description
<!-- add a short summary if necessary; mention issue numbers -->

This adds official support for the scenario where one towncrier config is shared between multiple projects with independent news files and fragments.

This is useful in monorepo-style setups.

# Checklist
<!-- add a "X" inside the brackets to confirm -->
* [x] Make sure changes are covered by existing or new tests.
* [x] For at least one Python version, make sure local test run is green.
* [x] 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).
* [x] 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.


iliakur: Gave it some more thought: this is looking even more like a Bugfix because `build` with the default `config.directory` supports the usecase as well. This PR only extends that support to custom `config.directory` values. 
adiroiban: Many thanks for the PR.

The thing is that normally an issue is created first.
As part of that issue, we can discuss if the current behaviour is the expect one, or a new feature is required or this is a bugfix.

with that, we can have a PR created and referencing the original issue.

As there is no issue, you can use 548 as the newsfragment / release notes id.

----

Is there already documentation for this functionality ?

If there is no documentation, this can be considered a new feature.

If there is documentation, but the sub-command don't work as documented, then we have a bug.
iliakur: @adiroiban Thanks for the quick response.

There is currently no documentation, **but** `towncrier add` supports the equivalent usecase. Moreover, `towncrier build` also supports this **as long as** `config.directory` has the default value. 

That's why it seems more like a bug to me than a missing feature. However, I'll defer to you for the final call, I'm also totally fine adding a feature-typed fragment.
adiroiban: Can you add some documentation as part of this PR.

The documentation should be the ultimate oracle telling what is a bug or a feature.

If something is not documented, then any change is a feature.
If something is already documented, but not working as documented, then we have a bug.

------

I guess that you try to use towncrier with a `monorepo` 

So I think that it make sense to add some information in the docs about how is the monorepo support implemented in `towncrier`.

Regards

iliakur: @adiroiban Thanks for the feedback I addressed it. In the process of adding the docs I realized that the `towncrier check` command also needed to be adjusted. With this scope this definitely feels more like a feature!

This PR is ready for review.
iliakur: > Thanks for the PR :)
> 
> I think the main thing here is that I am doing reviews at 1AM or during the day in 5 minutes intervals between the test runs of my main work :(
> 
> But I think we should merge this.
> 
> If anyone else is intersted into monorepo part, they can help improve the docs, the test or the configuration.
> 
> Please consider updating the tests to check for non-normalized path.
> 
> After that, I think that we can merge this.
> 
> Otherwise I don't know when I will have to review it or if someone else will have time soon to review this PR.

Let me know if you have a feeling that you'd want more time to consider this. We can also focus on 1 PR at a time and take a break with #553. 

I appreciate you carving out time to review and comment. 

I'll be addressing the part about non-normalized paths and CLI later today, so we should be good to merge over the weekend.
iliakur: @adiroiban thanks once more for bearing with this.

I'm a bit tired and sick today, I'll get to it tomorrow for sure. I'm thinking of moving all monorepo-related tests into one file so that it's easier to share setup code and comments for it. I want to test some more combinations of settings. Too bad `trial` doesn't support parametrized tests the way `pytest` does.
adiroiban: Hi. I hope you will get well soon.

We can merge this PR as it is. No need to have a perfect PR. As long as it's better that we have in `trunk` and doesn't introduce any regressions, it should be good to merge.

My comments were more of a "nice to have" category.

----------

I have never worked on mono-repos so I don't have experince with them.
So my feedback from this PR related to mono-repo might not be best.

This is why we can merge this and if someone else wants to use towncrier with monorepo, they can add more feedback or suggest changes.

---------

We can consider adding support for `parametrize` in trial.

Is this the feature ?

```
@pytest.mark.parametrize("test_input,expected", [("3+5", 8), ("2+4", 6), ("6*9", 42)])
def test_eval(test_input, expected):
    assert eval(test_input) == expected
```

I know that instead of simple `parametrize` in twisted/twisted we use https://hypothesis.readthedocs.io/en/latest/ ... not sure if it's the same thing.
iliakur: Alrighty, let's merge then :)

I owe you some docs and some tests.

---

To be precise:

- [ ] make sure cli and web docs are the same
- [ ] extend and refactor tests
- [ ] open issue base on one of your comments.

---

Your example with `pytest.parametrize` is correct. The concept is similar to `hypothesis.example`.

---

I have just one favor to ask of you: would it be possible to cut a release with this in the next couple days? That way I can incorporated it properly in our infrastructure (referencing branches isn't allowed in "prod").
adiroiban: > I owe you some docs and some tests.

You don't owe me anything :)

towncrier is just a project that I find useful, and was not mantained, so in my spare time I look after it.

we are standing on the shoulder of giants as many other people have contributed significant time for the project.

--------

> I have just one favor to ask of you: would it be possible to cut a release

Sure. Will do.

-------

I have updated the branch and enabled auto-merge . I hope this will merge soon :)

Patch:
diff --git a/docs/cli.rst b/docs/cli.rst
index 25eb2ce..90b5c89 100644
--- a/docs/cli.rst
+++ b/docs/cli.rst
@@ -12,7 +12,8 @@ The following options can be passed to all of the commands that explained below:
 
 .. option:: --dir PATH
 
-   Build fragment in ``PATH``.
+   The command is executed relative to ``PATH``.
+   For instance with the default config news fragments are checked and added in ``PATH/newsfragments`` and the news file is built in ``PATH/NEWS.rst``.
 
    Default: current directory.
 
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 7d73f50..d424e1c 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -51,12 +51,13 @@ Top level keys
     The directory storing your news fragments.
 
     For Python projects that provide a ``package`` key, the default is a ``newsfragments`` directory within the package.
-    Otherwise the default is a ``newsfragments`` directory relative to the configuration file.
+    Otherwise the default is a ``newsfragments`` directory relative to either the directory passed as ``--dir`` or (by default) the configuration file.
 
 ``filename``
     The filename of your news file.
 
     ``"NEWS.rst"`` by default.
+    Its location is determined the same way as the location of the directory storing the news fragments.
 
 ``template``
     Path to the template for generating the news file.
diff --git a/docs/index.rst b/docs/index.rst
index 975373d..f83e8ce 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -13,6 +13,7 @@ Narrative
 
    tutorial
    markdown
+   monorepo
 
 
 Reference
diff --git a/docs/monorepo.rst b/docs/monorepo.rst
new file mode 100644
index 0000000..0deb839
--- /dev/null
+++ b/docs/monorepo.rst
@@ -0,0 +1,52 @@
+Multiple Projects Share One Config (Monorepo)
+=============================================
+
+Several projects may have independent release notes with the same format.
+For instance packages in a monorepo.
+Here's how you can use towncrier to set this up.
+
+Below is a minimal example:
+
+.. code-block:: text
+
+  repo
+  ├── project_a
+  │   ├── newsfragments
+  │   │   └── 123.added
+  │   ├── project_a
+  │   │   └── __init__.py
+  │   └── NEWS.rst
+  ├── project_b
+  │   ├── newsfragments
+  │   │   └── 120.bugfix
+  │   ├── project_b
+  │   │   └── __init__.py
+  │   └── NEWS.rst
+  └── towncrier.toml
+
+The ``towncrier.toml`` looks like this:
+
+.. code-block:: toml
+
+  [tool.towncrier]
+  # It's important to keep these config fields empty
+  # because we have more than one package/name to manage.
+  package = ""
+  name = ""
+
+Now to add a fragment:
+
+.. code-block:: console
+
+   towncrier create --config towncrier.toml --dir project_a  124.added
+
+This should create a file at ``project_a/newsfragments/124.added``.
+
+To build the news file for the same project:
+
+.. code-block:: console
+
+   towncrier build --config towncrier.toml --dir project_a --version 1.5
+
+Note that we must explicitly pass ``--version``, there is no other way to get the version number.
+The ``towncrier.toml`` can only contain one version number and the ``package`` field is of no use for the same reason.
diff --git a/src/towncrier/build.py b/src/towncrier/build.py
index 60e0e86..3a07bae 100644
--- a/src/towncrier/build.py
+++ b/src/towncrier/build.py
@@ -175,7 +175,9 @@ def __main(
     click.echo("Finding news fragments...", err=to_err)
 
     if config.directory is not None:
-        fragment_base_directory = os.path.abspath(config.directory)
+        fragment_base_directory = os.path.abspath(
+            os.path.join(base_directory, config.directory)
+        )
         fragment_directory = None
     else:
         fragment_base_directory = os.path.abspath(
diff --git a/src/towncrier/check.py b/src/towncrier/check.py
index 2124590..ee9b612 100644
--- a/src/towncrier/check.py
+++ b/src/towncrier/check.py
@@ -93,9 +93,7 @@ def __main(
         )
         sys.exit(0)
 
-    files = {
-        os.path.normpath(os.path.join(base_directory, path)) for path in files_changed
-    }
+    files = {os.path.abspath(path) for path in files_changed}
 
     click.echo("Looking at these files:")
     click.echo("----")
@@ -109,7 +107,9 @@ def __main(
         sys.exit(0)
 
     if config.directory:
-        fragment_base_directory = os.path.abspath(config.directory)
+        fragment_base_directory = os.path.abspath(
+            os.path.join(base_directory, config.directory)
+        )
         fragment_directory = None
     else:
         fragment_base_directory = os.path.abspath(
@@ -118,7 +118,7 @@ def __main(
         fragment_directory = "newsfragments"
 
     fragments = {
-        os.path.normpath(path)
+        os.path.abspath(path)
         for path in find_fragments(
             fragment_base_directory,
             config.sections,
diff --git a/src/towncrier/newsfragments/548.feature b/src/towncrier/newsfragments/548.feature
new file mode 100644
index 0000000..c48f767
--- /dev/null
+++ b/src/towncrier/newsfragments/548.feature
@@ -0,0 +1,2 @@
+Initial support was added for monorepo-style setup.
+One project with multiple independent news files stored in separate sub-directories, that share the same towncrier config.


Test patch:
diff --git a/src/towncrier/test/test_build.py b/src/towncrier/test/test_build.py
index aa483ba..f15cd9a 100644
--- a/src/towncrier/test/test_build.py
+++ b/src/towncrier/test/test_build.py
@@ -145,6 +145,33 @@ class TestCli(TestCase):
         self.assertEqual(0, result.exit_code)
         self.assertTrue((project_dir / "NEWS.rst").exists())
 
+    @with_isolated_runner
+    def test_in_different_dir_with_nondefault_newsfragments_directory(self, runner):
+        """
+        Using the `--dir` CLI argument, the NEWS file can
+        be generated in a sub-directory from fragments
+        that are relatives to that sub-directory.
+
+        The path passed to `--dir` becomes the
+        working directory.
+        """
+        Path("pyproject.toml").write_text(
+            "[tool.towncrier]\n" + 'directory = "changelog.d"\n'
+        )
+        Path("foo/foo").mkdir(parents=True)
+        Path("foo/foo/__init__.py").write_text("")
+        Path("foo/changelog.d").mkdir()
+        Path("foo/changelog.d/123.feature").write_text("Adds levitation")
+        self.assertFalse(Path("foo/NEWS.rst").exists())
+
+        result = runner.invoke(
+            cli,
+            ("--yes", "--config", "pyproject.toml", "--dir", "foo", "--version", "1.0"),
+        )
+
+        self.assertEqual(0, result.exit_code)
+        self.assertTrue(Path("foo/NEWS.rst").exists())
+
     @with_isolated_runner
     def test_no_newsfragment_directory(self, runner):
         """
diff --git a/src/towncrier/test/test_check.py b/src/towncrier/test/test_check.py
index 102ab20..81861db 100644
--- a/src/towncrier/test/test_check.py
+++ b/src/towncrier/test/test_check.py
@@ -297,3 +297,101 @@ class TestChecker(TestCase):
 
         self.assertEqual("origin/master", branch)
         self.assertTrue(w[0].message.args[0].startswith('Using "origin/master'))
+
+    @with_isolated_runner
+    def test_in_different_dir_with_nondefault_newsfragments_directory(self, runner):
+        """
+        It can check the fragments located in a sub-directory
+        that is specified using the `--dir` CLI argument.
+        """
+        main_branch = "main"
+        Path("pyproject.toml").write_text(
+            # Important to customize `config.directory` because the default
+            # already supports this scenario.
+            "[tool.towncrier]\n"
+            + 'directory = "changelog.d"\n'
+        )
+        subproject1 = Path("foo")
+        (subproject1 / "foo").mkdir(parents=True)
+        (subproject1 / "foo/__init__.py").write_text("")
+        (subproject1 / "changelog.d").mkdir(parents=True)
+        (subproject1 / "changelog.d/123.feature").write_text("Adds levitation")
+        initial_commit(branch=main_branch)
+        call(["git", "checkout", "-b", "otherbranch"])
+
+        # We add a code change but forget to add a news fragment.
+        write(subproject1 / "foo/somefile.py", "import os")
+        commit("add a file")
+        result = runner.invoke(
+            towncrier_check,
+            (
+                "--config",
+                "pyproject.toml",
+                "--dir",
+                str(subproject1),
+                "--compare-with",
+                "main",
+            ),
+        )
+
+        self.assertEqual(1, result.exit_code)
+        self.assertTrue(
+            result.output.endswith("No new newsfragments found on this branch.\n")
+        )
+
+        # We add the news fragment.
+        fragment_path = (subproject1 / "changelog.d/124.feature").absolute()
+        write(fragment_path, "Adds gravity back")
+        commit("add a newsfragment")
+        result = runner.invoke(
+            towncrier_check,
+            ("--config", "pyproject.toml", "--dir", "foo", "--compare-with", "main"),
+        )
+
+        self.assertEqual(0, result.exit_code, result.output)
+        self.assertTrue(
+            result.output.endswith("Found:\n1. " + str(fragment_path) + "\n"),
+            (result.output, str(fragment_path)),
+        )
+
+        # We add a change in a different subproject without a news fragment.
+        # Checking subproject1 should pass.
+        subproject2 = Path("bar")
+        (subproject2 / "bar").mkdir(parents=True)
+        (subproject2 / "changelog.d").mkdir(parents=True)
+        write(subproject2 / "bar/somefile.py", "import os")
+        commit("add a file")
+        result = runner.invoke(
+            towncrier_check,
+            (
+                "--config",
+                "pyproject.toml",
+                "--dir",
+                subproject1,
+                "--compare-with",
+                "main",
+            ),
+        )
+
+        self.assertEqual(0, result.exit_code, result.output)
+        self.assertTrue(
+            result.output.endswith("Found:\n1. " + str(fragment_path) + "\n"),
+            (result.output, str(fragment_path)),
+        )
+
+        # Checking subproject2 should result in an error.
+        result = runner.invoke(
+            towncrier_check,
+            (
+                "--config",
+                "pyproject.toml",
+                "--dir",
+                subproject2,
+                "--compare-with",
+                "main",
+            ),
+        )
+        self.assertEqual(1, result.exit_code)
+        self.assertTrue(
+            result.output.endswith("No new newsfragments found on this branch.\n")
+        )
diff --git a/src/towncrier/test/test_create.py b/src/towncrier/test/test_create.py
index bb33da4..7f0e24b 100644
--- a/src/towncrier/test/test_create.py
+++ b/src/towncrier/test/test_create.py
@@ -249,3 +249,37 @@ class TestCli(TestCase):
         self.assertEqual(len(change.stem), 11)
         # Check the remainder are all hex characters.
         self.assertTrue(all(c in string.hexdigits for c in change.stem[3:]))
+
+    @with_isolated_runner
+    def test_in_different_dir_with_nondefault_newsfragments_directory(self, runner):
+        """
+        When the `--dir` CLI argument is passed,
+        it will create a new file in directory that is
+        created by combining the `--dir` value
+        with the `directory` option from the configuration
+        file.
+        """
+        Path("pyproject.toml").write_text(
+            # Important to customize `config.directory` because the default
+            # already supports this scenario.
+            "[tool.towncrier]\n"
+            + 'directory = "changelog.d"\n'
+        )
+        Path("foo/foo").mkdir(parents=True)
+        Path("foo/foo/__init__.py").write_text("")
+
+        result = runner.invoke(
+            _main,
+            (
+                "--config",
+                "pyproject.toml",
+                "--dir",
+                "foo",
+                "--content",
+                "Adds levitation.",
+                "123.feature",
+            ),
+        )
+
+        self.assertEqual(0, result.exit_code)
+        self.assertTrue(Path("foo/changelog.d/123.feature").exists())