jacebrowning / gitman

Language-agnostic dependency manager using Git.
https://gitman.readthedocs.io
MIT License
198 stars 32 forks source link

filter_nested_configs is doing the opposite of what it should #311

Closed davidwatkins-bdai closed 1 year ago

davidwatkins-bdai commented 1 year ago

When I introduced filter_nested_configs, I had meant to filter out any gitman.yml files that were present inside of the top-level config file. This requires more intelligently filtering any nested gitman.yml files that are present in already gitman install'd dependencies. This requires changing filter_nested_configs in $PROJECT_ROOT/models/config.py to:

def filter_nested_configs(
    configs: List[Config]
) -> List[Config]:
    """Filter subdirectories inside of parent config."""
    filtered_configs = []
    for config_a in configs:
        is_nested = False
        for config_b in configs:
            if config_a == config_b:
                continue
            if Path(config_b.location_path) in Path(config_a.location_path).parents:
                is_nested = True
                break
        if not is_nested:
            filtered_configs.append(config_a)

    return filtered_configs

And also modifying L91-L95 in $PROJECT_ROOT/gitman/commands.py to:

...
    config = load_config(root)
    configs = [config] if config else []
    configs.extend(find_nested_configs(root, depth, []))
    configs = filter_nested_configs(configs)
...
davidwatkins-bdai commented 1 year ago

@jacebrowning can I please get approval to make this PR?

jacebrowning commented 1 year ago

@davidwatkins-bdai Yeah. Since you said the logic is backwards can you add a test for this functionality?

davidwatkins-bdai commented 1 year ago

Absolutely. I wasn't able to push a branch to the repo to make the PR. Can you give me permission?

jacebrowning commented 1 year ago

https://github.com/davidwatkins has access. Do you have two accounts?

davidwatkins-bdai commented 1 year ago

I have this account for doing work-related activities and now I am making these adjustments using this one. I'll make the PR with my old account.

jacebrowning commented 1 year ago

This beta release contains the fix for this issue: https://pypi.org/project/gitman/3.4.1b1/