Skip to content
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

Ignore types on check #618

Merged
merged 4 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ These may include the following optional keys:
Orphan fragments (those without an issue number) always have their content included.
If a fragment was created, it means that information is important for end users.

``check``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not raising this in the first place. My bad.

Should this be more explicit / specific ?

Mabye in the future we would want to add some other things related to check behaviour. ... I am not sure what we might add... so this is just a minor comment.

Suggested change
``check``
``ignore_check``

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if this were ever needed, it would be easier to make the check a dict|bool rather than a separete knob from the ignore_check

[[tool.towncrier.type]]
...
check=false
[[tool.towncrier.type]]
...
check=true
[[tool.towncrier.type]]
...
check={"some_new_flag": "some_value"}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check={"some_new_flag": "some_value"}

this looks bad to me... if we use this, why bother with .INI or .TOML and not just go with JSON ?

If you dislike ignore_check then we can keep the current option.

A boolean value indicating whether the fragment should be considered by the ``towncrier check`` command.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not very clear... but I don't know how to better explain ths... writing good documentation is hard :)

Maybe

Suggested change
A boolean value indicating whether the fragment should be considered by the ``towncrier check`` command.
A boolean value indicating whether the fragment should be considered by the ``towncrier check`` command.
When set to `false`, ``towncrier check`` will fail, unless the branch contains other newsfragment types.


``true`` by default.

For example, if you want your custom fragment types to be ``["feat", "fix", "chore",]`` and you want all of them to use the default configuration except ``"chore"`` you can do it as follows:

.. code-block:: toml
Expand All @@ -234,6 +239,10 @@ For example, if you want your custom fragment types to be ``["feat", "fix", "cho
name = "Other Tasks"
showcontent = false

[tool.towncrier.fragment.auto]
name = "Automatic Changes"
check = false


.. warning::

Expand Down Expand Up @@ -267,6 +276,11 @@ Each table within this array has the following mandatory keys:
Orphan fragments (those without an issue number) always have their content included.
If a fragment was created, it means that information is important for end users.

``check``
A boolean value indicating whether the fragment should be considered by the ``towncrier check`` command.

``true`` by default.

For example:

.. code-block:: toml
Expand All @@ -281,3 +295,9 @@ For example:
directory = "chore"
name = "Other Tasks"
showcontent = false

[[tool.towncrier.type]]
directory = "auto"
name = "Automatic Changes"
adiroiban marked this conversation as resolved.
Show resolved Hide resolved
showcontent = true
check = false
9 changes: 6 additions & 3 deletions src/towncrier/_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ def __call__(self, section_directory: str = "") -> str:
#
# {
# "": {
# ("142", "misc"): "",
# ("1", "feature"): "some cool description",
# ("142", "misc", 1): "",
# ("1", "feature", 1): "some cool description",
# },
# "Names": {},
# "Web": {("3", "bugfix"): "Fixed a thing"},
# "Web": {("3", "bugfix", 1): "Fixed a thing"},
# }
#
# We should really use attrs.
Expand All @@ -102,6 +102,7 @@ def __call__(self, section_directory: str = "") -> str:
def find_fragments(
base_directory: str,
config: Config,
only_check_categories: bool = False,
adiroiban marked this conversation as resolved.
Show resolved Hide resolved
) -> tuple[Mapping[str, Mapping[tuple[str, str, int], str]], list[str]]:
"""
Sections are a dictonary of section names to paths.
Expand Down Expand Up @@ -130,6 +131,8 @@ def find_fragments(
)
if category is None:
continue
if only_check_categories and config.types[category]["check"] is False:
continue
assert issue is not None
assert counter is not None
if config.orphan_prefix and issue.startswith(config.orphan_prefix):
Expand Down
18 changes: 13 additions & 5 deletions src/towncrier/_settings/fragment_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,15 @@ class DefaultFragmentTypesLoader(BaseFragmentTypesLoader):

_default_types = {
# Keep in-sync with docs/tutorial.rst.
"feature": {"name": "Features", "showcontent": True},
"bugfix": {"name": "Bugfixes", "showcontent": True},
"doc": {"name": "Improved Documentation", "showcontent": True},
"removal": {"name": "Deprecations and Removals", "showcontent": True},
"misc": {"name": "Misc", "showcontent": False},
"feature": {"name": "Features", "showcontent": True, "check": True},
"bugfix": {"name": "Bugfixes", "showcontent": True, "check": True},
"doc": {"name": "Improved Documentation", "showcontent": True, "check": True},
"removal": {
"name": "Deprecations and Removals",
"showcontent": True,
"check": True,
},
"misc": {"name": "Misc", "showcontent": False, "check": True},
}

def load(self) -> Mapping[str, Mapping[str, Any]]:
Expand Down Expand Up @@ -75,9 +79,11 @@ def load(self) -> Mapping[str, Mapping[str, Any]]:
directory = type_config["directory"]
fragment_type_name = type_config["name"]
is_content_required = type_config["showcontent"]
check = type_config.get("check", True)
types[directory] = {
"name": fragment_type_name,
"showcontent": is_content_required,
"check": check,
}
return types

Expand Down Expand Up @@ -129,8 +135,10 @@ def _load_options(self, fragment_type: str) -> Mapping[str, Any]:
options = self.fragment_options.get(fragment_type, {})
fragment_description = options.get("name", capitalized_fragment_type)
show_content = options.get("showcontent", True)
check = options.get("check", True)
clean_fragment_options = {
"name": fragment_description,
"showcontent": show_content,
"check": check,
}
return clean_fragment_options
5 changes: 4 additions & 1 deletion src/towncrier/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ def __main(
sys.exit(0)

fragments = {
os.path.abspath(path) for path in find_fragments(base_directory, config)[1]
os.path.abspath(path)
for path in find_fragments(base_directory, config, only_check_categories=True)[
1
]
}
fragments_in_branch = fragments & files

Expand Down
1 change: 1 addition & 0 deletions src/towncrier/newsfragments/617.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
newsfragment categories can now be marked with ``check = false``, causing them to be ignored in ``towncrier check``
65 changes: 65 additions & 0 deletions src/towncrier/test/test_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,71 @@ def test_fragment_missing(self):
result.output.endswith("No new newsfragments found on this branch.\n")
)

def test_fragment_exists_but_not_in_check(self):
adiroiban marked this conversation as resolved.
Show resolved Hide resolved
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 = "auto"\n'
'name = "Automatic"\n'
adiroiban marked this conversation as resolved.
Show resolved Hide resolved
"showcontent = true\n"
"check=false\n",
)

file_path = "foo/somefile.py"
write(file_path, "import os")

fragment_path = Path("foo/newsfragments/1234.auto").absolute()
write(fragment_path, "Adds gravity back")
commit("add a file and a newsfragment")

result = runner.invoke(towncrier_check, ["--compare-with", "master"])

self.assertEqual(1, result.exit_code)
self.assertTrue(
result.output.endswith("No new newsfragments found on this branch.\n")
)

def test_fragment_exists_but_in_check(self):
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 = "auto"\n'
'name = "Automatic"\n'
"showcontent = true\n"
"check=false\n",
)

file_path = "foo/somefile.py"
write(file_path, "import os")

fragment_path = Path("foo/newsfragments/1234.feature").absolute()
write(fragment_path, "Adds gravity back")
commit("add a file and a newsfragment")

result = runner.invoke(towncrier_check, ["--compare-with", "master"])

self.assertEqual(0, result.exit_code)
self.assertTrue(
result.output.endswith("Found:\n1. " + str(fragment_path) + "\n"),
(result.output, str(fragment_path)),
)

def test_none_stdout_encoding_works(self):
"""
No failure when output is piped causing None encoding for stdout.
Expand Down
27 changes: 27 additions & 0 deletions src/towncrier/test/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,12 @@ def test_custom_types_as_tables_array_deprecated(self):
directory="spam"
name="Spam"
showcontent=true

[[tool.towncrier.type]]
directory="auto"
name="Automatic"
showcontent=true
check=false
"""
)
config = load_config(project_dir)
Expand All @@ -297,13 +303,23 @@ def test_custom_types_as_tables_array_deprecated(self):
{
"name": "Foo",
"showcontent": False,
"check": True,
},
),
(
"spam",
{
"name": "Spam",
"showcontent": True,
"check": True,
},
),
(
"auto",
{
"name": "Automatic",
"showcontent": True,
"check": False,
},
),
]
Expand All @@ -326,21 +342,32 @@ def test_custom_types_as_tables(self):
[tool.towncrier.fragment.chore]
name = "Other Tasks"
showcontent = false
[tool.towncrier.fragment.auto]
name = "Automatic"
check = false
"""
)
config = load_config(project_dir)
expected = {
"chore": {
"name": "Other Tasks",
"showcontent": False,
"check": True,
},
"feat": {
"name": "Feat",
"showcontent": True,
"check": True,
},
"fix": {
"name": "Fix",
"showcontent": True,
"check": True,
},
"auto": {
"name": "Automatic",
"showcontent": True,
"check": False,
},
}
actual = config.types
Expand Down
Loading