Changes the structure of resolution packages

[m1kola]

Description

Follow up from this discussion: moving things around in internal/resolution.

Before:

internal/resolution
├── entitysources
│   └── catalogdsource.go
├── resolver_test.go
└── variable_sources
    ├── bundlesanddependencies
    │   ├── bundles_and_dependencies.go
    │   └── bundles_and_dependencies_test.go
    ├── crdconstraints
    │   ├── crd_constraints.go
    │   └── crd_constraints_test.go
    ├── entity
    │   ├── bundle_entity.go
    │   └── bundle_entity_test.go
    ├── olm
    │   ├── olm.go
    │   └── olm_test.go
    ├── requiredpackage
    │   ├── required_package.go
    │   └── required_package_test.go
    └── util
        ├── predicates
        │   ├── predicates.go
        │   └── predicates_test.go
        └── sort
            ├── sort.go
            └── sort_test.go

11 directories, 16 files

After:

internal/resolution
├── entities
│   ├── bundle_entity.go
│   └── bundle_entity_test.go
├── entitysources
│   └── catalogdsource.go
├── resolver_test.go
├── util
│   ├── predicates
│   │   ├── predicates.go
│   │   └── predicates_test.go
│   └── sort
│       ├── sort.go
│       └── sort_test.go
├── variables
│   ├── bundle.go
│   ├── bundle_test.go
│   ├── required_package.go
│   ├── required_package_test.go
│   └── variables_test.go
└── variablesources
    ├── bundles_and_dependencies.go
    ├── bundles_and_dependencies_test.go
    ├── crd_constraints.go
    ├── crd_constraints_test.go
    ├── operator.go
    ├── operator_test.go
    ├── required_package.go
    ├── required_package_test.go
    └── variablesources_test.go

8 directories, 22 files

Reviewer Checklist

• API Go Documentation
• Tests: Unit Tests (and E2E Tests, if appropriate)
• Comprehensive Commit Messages
• Links to related GitHub Issue(s)

[m1kola]

This previously was internal/resolution/variable_sources/olm/olm.go, but due to the amount of renames in the file git considers it to be a new file.

[tmshort]

Did you use git mv in any of this?

[m1kola]

@tmshort Yes, I did. But when you start modifying moved files - at some point git considers original files deleted and new location as added. One option to overcome this is to move files, commit them and then fix compilation errors in a separate commit. But if you ever need to git bisect for debugging - it messes it up.

[everettraven]

This looks much better IMO.

I do have a nit - is there a logical way to break down the variablesources package a bit more?

It seems like it contains variable sources, variables, and constraints. Maybe it makes sense to break each of them out into their own packages just for some slightly more intuitive directory navigation? We already seem to be following this pattern for entities and entitysources so it would also be a bit more consistent.

[m1kola]

@everettraven there are no contstraints in variablesources (nothing which implements deppy.Constraint interface), but I agree that there is an opportunity to split variables and variable sources. I gave it a go in a separate commit. Please take another look.

[tmshort]

To make sure I know what you're doing. There is a difference of 6 files (16 in the old tree, 22 in the new)

Unmoved: (2)

• entitysources/catalogdsource.go
• resolver_test.go

Moved: (12)

• variable_sources/bundlesanddependencies/bundles_and_dependencies.go -> variablesource/bundles_and_dependencies.go (and _test.go)
• variable_sources/crdconstraints/crd_constraints.go -> variablesource/crd_constraints.go (and _test.go)
• variable_sources/entity/bundle_entity.go -> entities/bundle_entity.go (and _test.go)
• variable_sources/olm/olm.go -> variablesources/operator.go (and _test.go)
• variable_sources/util/... -> util/...

New (+4):

• variables/variables_test.go
• variablesources/variablesources_test.go
• variables/bundle.go (and _test.go)

Split/copied?? (2 vs 4)

• variable_sources/requiredpackage/required_package.go (and _test.go)
  • -> variables/required_package.go
  • -> variablesources/required_package.go

[m1kola]

@tmshort Yes, I think your summary is mostly accurate apart from the variablesoruce/variable split (see 2f7f148).

• variable_sources/crdconstraints/crd_constraints.go:
  • Variable ended up in variables/bundle.go (and _test.go for it)
  • Variable source in variablesource/crd_constraints.go (and _test.go for it)
• variable_sources/bundlesanddependencies/bundles_and_dependencies.go:
  • Variable ended up in variables/bundle.go (and _test.go for it)
  • Variable source in variablesource/bundles_and_dependencies.go (and _test.go for it)
• variable_sources/requiredpackage/required_package.go:
  • Variable ended up in variables/required_package.go (and _test.go for it)
  • Variable source in variablesources/required_package.go (and _test.go for it)

variables/variables_test.go and variablesources/variablesources_test.go are just to enable ginkgo tests. I suspect they will go away once we complete #189

[tmshort]

Ah, I think I get the distinction slightly better now.

[m1kola]

Rebased to fix conflicts in internal/resolution/variablesources/bundles_and_dependencies.go and internal/resolution/variablesources/crd_constraints_test.go after merging #272.

Please take another look.

[tmshort]

Looks like everything is there and moved around a bit. Some renaming.
/lgtm