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

Allow towncrier to traverse back up directories looking for the configuration file #601

Merged
merged 9 commits into from
May 22, 2024
37 changes: 35 additions & 2 deletions src/towncrier/_settings/load.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,26 +65,59 @@ def __init__(self, *args: str, **kwargs: str):
def load_config_from_options(
directory: str | None, config_path: str | None
) -> tuple[str, Config]:
"""
Load the configuration from the given directory or specific configuration file.

Returns a tuple of the base directory and the parsed Config instance.
"""
if config_path is None:
if directory is None:
directory = os.getcwd()
# Neither a directory or explicit config path was given so traverse for a
# config.
return traverse_for_config(None)
Copy link
Member

Choose a reason for hiding this comment

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

I think that this should be private.

I know that this is from _settings.load , but I think that we should make it explicit that we don't want to expose this as a public API.

Also, there are a lot of branches here,
so I think that it would help to add an explicit comment for this branch.

With the missing docstring for the load_config_from_options it's a bit hard to understand the purpose of this function.

Can we have something like this.

It lools like a bit of duplication between
traverse_for_config and these following lines

base_directory = os.path.abspath(directory)
config = load_config(base_directory)

maybe have something like this

    if config_path is None:
        return _traverse_for_config(directory)

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 don't think there's any assumption that non underscored methods become automatically part of a public API is there? If people want to shoot themselves in the foot, let em 🤠

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've added a docstring and some comments to explain the different branches (I don't like the complexity of the towncrier's loading, but I'm not going to rewrite it just for this addition)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I see what you're getting at with the branching logic now. I've simplified it (and added another test).


# Since no explicit config path was provided, we'll look for a config in the
# given directory.
base_directory = os.path.abspath(directory)
config = load_config(base_directory)
else:
# If an explicit config path was provided, we'll use that.
config_path = os.path.abspath(config_path)
config = load_config_from_file(os.path.dirname(config_path), config_path)

# If a directory was also provided, we'll use that as the base directory
# otherwise we'll use the directory containing the config file.
if directory is None:
base_directory = os.path.dirname(config_path)
else:
base_directory = os.path.abspath(directory)
config = load_config_from_file(os.path.dirname(config_path), config_path)

if config is None:
raise ConfigError(f"No configuration file found.\nLooked in: {base_directory}")

return base_directory, config


def traverse_for_config(path: str | None) -> tuple[str, Config]:
"""
Search for a configuration file in the current directory and all parent directories.

Returns the directory containing the configuration file and the parsed configuration.
"""
start_directory = directory = os.path.abspath(path or os.getcwd())
while True:
config = load_config(directory)
if config is not None:
return directory, config

parent = os.path.dirname(directory)
if parent == directory:
raise ConfigError(
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
f"No configuration file found.\nLooked back from: {start_directory}"
)
directory = parent


def load_config(directory: str) -> Config | None:
towncrier_toml = os.path.join(directory, "towncrier.toml")
pyproject_toml = os.path.join(directory, "pyproject.toml")
Expand Down
1 change: 1 addition & 0 deletions src/towncrier/newsfragments/601.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Running ``towncrier`` will now traverse back up directories looking for the configuration file.
12 changes: 11 additions & 1 deletion src/towncrier/test/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,21 @@ def test_in_different_dir_dir_option(self, runner):
self.assertEqual(0, result.exit_code)
self.assertTrue((project_dir / "NEWS.rst").exists())

@with_project()
def test_traverse_up_to_find_config(self, runner):
"""
If the current directory doesn't contain the configuration file, Towncrier
should traverse up the directory tree until it finds it.
SmileyChris marked this conversation as resolved.
Show resolved Hide resolved
"""
os.chdir("foo")
result = runner.invoke(_main, ["--draft", "--date", "01-01-2001"])
self.assertEqual(0, result.exit_code, result.output)

@with_project()
def test_in_different_dir_config_option(self, runner):
"""
The current working directory and the location of the configuration
don't matter as long as we pass corrct paths to the directory and the
don't matter as long as we pass correct paths to the directory and the
config file.
"""
project_dir = Path(".").resolve()
Expand Down
Loading