From 9832f0400596acbfa0d2bb626980dd67cf213be3 Mon Sep 17 00:00:00 2001 From: "Patterson, Kevin R" Date: Tue, 3 Dec 2024 18:30:55 -0600 Subject: [PATCH 1/6] add an option to check staged files --- src/towncrier/_git.py | 15 +++++-- src/towncrier/check.py | 21 +++++++-- src/towncrier/newsfragments/676.feature.rst | 1 + src/towncrier/test/test_check.py | 47 +++++++++++++++++++++ 4 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 src/towncrier/newsfragments/676.feature.rst diff --git a/src/towncrier/_git.py b/src/towncrier/_git.py index ff3d5448..457e70f4 100644 --- a/src/towncrier/_git.py +++ b/src/towncrier/_git.py @@ -41,7 +41,7 @@ def get_remote_branches(base_directory: str) -> list[str]: def list_changed_files_compared_to_branch( - base_directory: str, compare_with: str + base_directory: str, compare_with: str, include_staged: bool ) -> list[str]: output = check_output( ["git", "diff", "--name-only", compare_with + "..."], @@ -49,5 +49,14 @@ def list_changed_files_compared_to_branch( encoding="utf-8", stderr=STDOUT, ) - - return output.strip().splitlines() + filenames = output.strip().splitlines() + if include_staged: + output = check_output( + ["git", "diff", "--name-only", "--cached"], + cwd=base_directory, + encoding="utf-8", + stderr=STDOUT, + ) + filenames.extend(output.strip().splitlines()) + + return filenames diff --git a/src/towncrier/check.py b/src/towncrier/check.py index 8b057545..936751b9 100644 --- a/src/towncrier/check.py +++ b/src/towncrier/check.py @@ -57,15 +57,28 @@ def _get_default_compare_branch(branches: Container[str]) -> str | None: metavar="FILE_PATH", help=config_option_help, ) -def _main(compare_with: str | None, directory: str | None, config: str | None) -> None: +@click.option( + "--staged", + "staged", + is_flag=True, + default=False, + metavar="STAGED", + help="include staged files as part of the branch checked in the --compare-with", +) +def _main( + compare_with: str | None, directory: str | None, config: str | None, staged: bool +) -> None: """ Check for new fragments on a branch. """ - __main(compare_with, directory, config) + __main(compare_with, directory, config, staged) def __main( - comparewith: str | None, directory: str | None, config_path: str | None + comparewith: str | None, + directory: str | None, + config_path: str | None, + staged: bool, ) -> None: base_directory, config = load_config_from_options(directory, config_path) @@ -80,7 +93,7 @@ def __main( try: files_changed = list_changed_files_compared_to_branch( - base_directory, comparewith + base_directory, comparewith, staged ) except CalledProcessError as e: click.echo("git produced output while failing:") diff --git a/src/towncrier/newsfragments/676.feature.rst b/src/towncrier/newsfragments/676.feature.rst new file mode 100644 index 00000000..740a8e6f --- /dev/null +++ b/src/towncrier/newsfragments/676.feature.rst @@ -0,0 +1 @@ +new `--staged` parameter for the `check` to included staged files as part of confirming a news file exists diff --git a/src/towncrier/test/test_check.py b/src/towncrier/test/test_check.py index 9d8c05aa..676661c1 100644 --- a/src/towncrier/test/test_check.py +++ b/src/towncrier/test/test_check.py @@ -41,6 +41,15 @@ def commit(message): call(["git", "commit", "-m", message]) +def stage(): + """Stage a commit to the repo in the current working directory + + There must be uncommitted changes otherwise git will complain: + "nothing to commit, working tree clean" + """ + call(["git", "add", "."]) + + def initial_commit(branch="main"): """ Create a git repo, configure it and make an initial commit @@ -204,6 +213,44 @@ def test_fragment_exists_but_not_in_check(self): (result.output, str(fragment_path)), ) + def test_fragment_exists_and_staged(self): + """A fragment that exists but is marked as check=False is ignored by the check.""" + runner = CliRunner() + + with runner.isolated_filesystem(): + create_project( + "pyproject.toml", + main_branch="master", + extra_config="[[tool.towncrier.type]]\n" + 'directory = "feature"\n' + 'name = "Features"\n' + "showcontent = true\n" + "[[tool.towncrier.type]]\n" + 'directory = "sut"\n' + 'name = "System Under Test"\n' + "showcontent = true\n" + "check=false\n", + ) + + file_path = "foo/somefile.py" + write(file_path, "import os") + + fragment_path = Path("foo/newsfragments/1234.sut").absolute() + write(fragment_path, "Not really a fragment") + commit("add some files that mean we should have a fragment") + + fragment_path = Path("foo/newsfragments/1234.feature").absolute() + write(fragment_path, "Adds gravity back") + stage() + + result = runner.invoke(towncrier_check, ["--compare-with", "master"]) + + self.assertEqual(1, result.exit_code) + result = runner.invoke( + towncrier_check, ["--staged", "--compare-with", "master"] + ) + self.assertEqual(0, result.exit_code) + def test_fragment_exists_and_in_check(self): """ A fragment that exists but is not marked as check=False is From 69c78a64e8f1fd012ccc92b8161c9d16bcfa7ee4 Mon Sep 17 00:00:00 2001 From: "Patterson, Kevin R" Date: Tue, 3 Dec 2024 18:40:41 -0600 Subject: [PATCH 2/6] add CLI doc update --- docs/cli.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/cli.rst b/docs/cli.rst index c8040461..d462350a 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -121,3 +121,10 @@ By default, ``towncrier`` compares the current branch against ``origin/main`` (a Use ``REMOTE-BRANCH`` instead of ``origin/main``:: $ towncrier check --compare-with origin/trunk + +.. option:: --staged + + Include files that have been staged for commit when checking for news fragments + + $ towncrier check --staged + $ towncrier check --staged --compare-with origin/trunk From d9f72885d3a5272e961caa8b3c56ac881228cdf5 Mon Sep 17 00:00:00 2001 From: Kevin Patterson Date: Wed, 4 Dec 2024 07:30:43 -0600 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Tom Most --- docs/cli.rst | 2 +- src/towncrier/check.py | 2 +- src/towncrier/newsfragments/676.feature.rst | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/cli.rst b/docs/cli.rst index d462350a..12baf1d8 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -124,7 +124,7 @@ By default, ``towncrier`` compares the current branch against ``origin/main`` (a .. option:: --staged - Include files that have been staged for commit when checking for news fragments + Include files that have been staged for commit when checking for news fragments:: $ towncrier check --staged $ towncrier check --staged --compare-with origin/trunk diff --git a/src/towncrier/check.py b/src/towncrier/check.py index 936751b9..c6a29586 100644 --- a/src/towncrier/check.py +++ b/src/towncrier/check.py @@ -63,7 +63,7 @@ def _get_default_compare_branch(branches: Container[str]) -> str | None: is_flag=True, default=False, metavar="STAGED", - help="include staged files as part of the branch checked in the --compare-with", + help="Include staged files as part of the branch checked in the --compare-with", ) def _main( compare_with: str | None, directory: str | None, config: str | None, staged: bool diff --git a/src/towncrier/newsfragments/676.feature.rst b/src/towncrier/newsfragments/676.feature.rst index 740a8e6f..a0e0cfff 100644 --- a/src/towncrier/newsfragments/676.feature.rst +++ b/src/towncrier/newsfragments/676.feature.rst @@ -1 +1 @@ -new `--staged` parameter for the `check` to included staged files as part of confirming a news file exists +The `towncrier check` command now has a `--staged` flag to inspect the files staged for commit when checking for a news fragment: useful in a pre-commit hook From 5b5c8ca5b5438dc5db8d212becc068b1effe56c7 Mon Sep 17 00:00:00 2001 From: "Patterson, Kevin R" Date: Wed, 4 Dec 2024 07:38:43 -0600 Subject: [PATCH 4/6] change all calls to check_call --- src/towncrier/test/test_check.py | 40 +++++++++++++++++--------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/towncrier/test/test_check.py b/src/towncrier/test/test_check.py index 676661c1..e1aecbc5 100644 --- a/src/towncrier/test/test_check.py +++ b/src/towncrier/test/test_check.py @@ -6,7 +6,7 @@ import warnings from pathlib import Path -from subprocess import call +from subprocess import check_call from click.testing import CliRunner from twisted.trial.unittest import TestCase @@ -28,7 +28,7 @@ def create_project( setup_simple_project(pyproject_path=pyproject_path, extra_config=extra_config) Path("foo/newsfragments/123.feature").write_text("Adds levitation") initial_commit(branch=main_branch) - call(["git", "checkout", "-b", "otherbranch"]) + check_call(["git", "checkout", "-b", "otherbranch"]) def commit(message): @@ -37,8 +37,8 @@ def commit(message): There must be uncommitted changes otherwise git will complain: "nothing to commit, working tree clean" """ - call(["git", "add", "."]) - call(["git", "commit", "-m", message]) + check_call(["git", "add", "."]) + check_call(["git", "commit", "-m", message]) def stage(): @@ -47,7 +47,7 @@ def stage(): There must be uncommitted changes otherwise git will complain: "nothing to commit, working tree clean" """ - call(["git", "add", "."]) + check_call(["git", "add", "."]) def initial_commit(branch="main"): @@ -59,11 +59,11 @@ def initial_commit(branch="main"): """ # --initial-branch is explicitly set to `main` because # git has deprecated the default branch name. - call(["git", "init", f"--initial-branch={branch}"]) + check_call(["git", "init", f"--initial-branch={branch}"]) # Without ``git config` user.name and user.email `git commit` fails # unless the settings are set globally - call(["git", "config", "user.name", "user"]) - call(["git", "config", "user.email", "user@example.com"]) + check_call(["git", "config", "user.name", "user"]) + check_call(["git", "config", "user.email", "user@example.com"]) commit("Initial Commit") @@ -165,8 +165,8 @@ def test_fragment_missing(self): with open(file_path, "w") as f: f.write("import os") - call(["git", "add", "foo/somefile.py"]) - call(["git", "commit", "-m", "add a file"]) + check_call(["git", "add", "foo/somefile.py"]) + check_call(["git", "commit", "-m", "add a file"]) result = runner.invoke(towncrier_check, ["--compare-with", "master"]) @@ -301,8 +301,8 @@ def test_none_stdout_encoding_works(self): with open(fragment_path, "w") as f: f.write("Adds gravity back") - call(["git", "add", fragment_path]) - call(["git", "commit", "-m", "add a newsfragment"]) + check_call(["git", "add", fragment_path]) + check_call(["git", "commit", "-m", "add a newsfragment"]) runner = CliRunner(mix_stderr=False) result = runner.invoke(towncrier_check, ["--compare-with", "master"]) @@ -357,16 +357,18 @@ def test_release_branch(self): commit("First release") # The news file is now created. self.assertIn("NEWS.rst", os.listdir(".")) - call(["git", "checkout", "main"]) - call(["git", "merge", "otherbranch", "-m", "Sync release in main branch."]) + check_call(["git", "checkout", "main"]) + check_call( + ["git", "merge", "otherbranch", "-m", "Sync release in main branch."] + ) # We have a new feature branch that has a news fragment that # will be merged to the main branch. - call(["git", "checkout", "-b", "new-feature-branch"]) + check_call(["git", "checkout", "-b", "new-feature-branch"]) write("foo/newsfragments/456.feature", "Foo the bar") commit("A feature in the second release.") - call(["git", "checkout", "main"]) - call( + check_call(["git", "checkout", "main"]) + check_call( [ "git", "merge", @@ -377,7 +379,7 @@ def test_release_branch(self): ) # We now have the new release branch. - call(["git", "checkout", "-b", "next-release"]) + check_call(["git", "checkout", "-b", "next-release"]) runner.invoke(towncrier_build, ["--yes", "--version", "2.0"]) commit("Second release") @@ -439,7 +441,7 @@ def test_in_different_dir_with_nondefault_newsfragments_directory(self, runner): (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"]) + check_call(["git", "checkout", "-b", "otherbranch"]) # We add a code change but forget to add a news fragment. write(subproject1 / "foo/somefile.py", "import os") From 363532aa92fc31c5d46c6bea2cd4760a7604c4f9 Mon Sep 17 00:00:00 2001 From: "Patterson, Kevin R" Date: Wed, 4 Dec 2024 07:44:12 -0600 Subject: [PATCH 5/6] remove unnecessary lines from the staging test --- src/towncrier/test/test_check.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/towncrier/test/test_check.py b/src/towncrier/test/test_check.py index e1aecbc5..bfb76191 100644 --- a/src/towncrier/test/test_check.py +++ b/src/towncrier/test/test_check.py @@ -214,7 +214,7 @@ def test_fragment_exists_but_not_in_check(self): ) def test_fragment_exists_and_staged(self): - """A fragment that exists but is marked as check=False is ignored by the check.""" + """A fragment exists and is added in staging. Pass only if staging on the command line""" runner = CliRunner() with runner.isolated_filesystem(): @@ -228,16 +228,13 @@ def test_fragment_exists_and_staged(self): "[[tool.towncrier.type]]\n" 'directory = "sut"\n' 'name = "System Under Test"\n' - "showcontent = true\n" - "check=false\n", + "showcontent = true\n", ) file_path = "foo/somefile.py" write(file_path, "import os") - fragment_path = Path("foo/newsfragments/1234.sut").absolute() - write(fragment_path, "Not really a fragment") - commit("add some files that mean we should have a fragment") + commit("add some files for test initialization") fragment_path = Path("foo/newsfragments/1234.feature").absolute() write(fragment_path, "Adds gravity back") From de88e69f9eb2aecfefcfe259250d093a2bf2dcc9 Mon Sep 17 00:00:00 2001 From: "Patterson, Kevin R" Date: Wed, 4 Dec 2024 07:45:57 -0600 Subject: [PATCH 6/6] metavar is unused for booleans --- src/towncrier/check.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/towncrier/check.py b/src/towncrier/check.py index c6a29586..bf3c6e2f 100644 --- a/src/towncrier/check.py +++ b/src/towncrier/check.py @@ -62,7 +62,6 @@ def _get_default_compare_branch(branches: Container[str]) -> str | None: "staged", is_flag=True, default=False, - metavar="STAGED", help="Include staged files as part of the branch checked in the --compare-with", ) def _main(