Skip to content

Commit

Permalink
refactor: Rename ManifestLoaderContext and document symbols
Browse files Browse the repository at this point in the history
Best practice in Go is not to have the package name as a prefix, since it is redundant information in the usage.
  • Loading branch information
Laubi committed Mar 20, 2023
1 parent 1241592 commit 70089b3
Show file tree
Hide file tree
Showing 14 changed files with 26 additions and 22 deletions.
2 changes: 1 addition & 1 deletion cmd/monaco/delete/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func Delete(fs afero.Fs, deploymentManifestPath string, deleteFile string, envir

apis := api.NewAPIs()

manifest, manifestLoadError := manifest.LoadManifest(&manifest.ManifestLoaderContext{
manifest, manifestLoadError := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: deploymentManifestPath,
Environments: environmentNames,
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func absPath(manifestPath string) (string, error) {
}

func loadManifest(fs afero.Fs, manifestPath string, groups []string, environments []string) (*manifest.Manifest, error) {
m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
m, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: manifestPath,
Groups: groups,
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/download/download_configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type directDownloadOptions struct {

func (d DefaultCommand) DownloadConfigsBasedOnManifest(fs afero.Fs, cmdOptions manifestDownloadOptions) error {

m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
m, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: cmdOptions.manifestFile,
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/download/download_entities.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type entitiesDirectDownloadOptions struct {

func (d DefaultCommand) DownloadEntitiesBasedOnManifest(fs afero.Fs, cmdOptions entitiesManifestDownloadOptions) error {

m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
m, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: cmdOptions.manifestFile,
})
Expand Down
4 changes: 2 additions & 2 deletions cmd/monaco/download/download_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ func TestDownloadIntegrationOverwritesFolderAndManifestIfForced(t *testing.T) {
assert.NilError(t, err)

// THEN we can load the project again and verify its content
man, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
man, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: filepath.Join(testBasePath, "manifest.yaml"),
})
Expand Down Expand Up @@ -1013,7 +1013,7 @@ func setupTestingDownloadOptions(t *testing.T, server *httptest.Server, projectN
}

func loadDownloadedProjects(fs afero.Fs, apis api.APIs) ([]projectLoader.Project, []error) {
man, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
man, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: "out/manifest.yaml",
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/integrationtest/integration_test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func LoadManifest(t *testing.T, fs afero.Fs, manifestFile string, specificEnviro
specificEnvs = append(specificEnvs, specificEnvironment)
}

m, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
m, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: manifestFile,
Environments: specificEnvs,
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/integrationtest/v1/integration_test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func runLegacyIntegration(t *testing.T, configFolder, envFile, suffixTest string
assert.NilError(t, err)
assert.Assert(t, exists, "manifest should exist on path '%s' but does not", manifestPath)

loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: manifestPath,
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/integrationtest/v2/cleanup_all_configs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestDoCleanup(t *testing.T) {
manifestPath := "test-resources/test_environments_manifest.yaml"

fs := afero.NewOsFs()
loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: manifestPath,
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/integrationtest/v2/config_restore_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ func validation_uploadDownloadedConfigs(t *testing.T, fs afero.Fs, downloadFolde
}

func cleanupDeployedConfiguration(t *testing.T, fs afero.Fs, manifestFilepath string, testSuffix string) {
loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: manifestFilepath,
})
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/integrationtest/v2/integration_test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func runIntegrationWithCleanup(t *testing.T, testFs afero.Fs, configFolder, mani
envs = append(envs, specificEnvironment)
}

loadedManifest, errs := manifest.LoadManifest(&manifest.ManifestLoaderContext{
loadedManifest, errs := manifest.LoadManifest(&manifest.LoaderContext{
Fs: testFs,
ManifestPath: manifestPath,
Environments: envs,
Expand Down
2 changes: 1 addition & 1 deletion cmd/monaco/purge/purge.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func purge(fs afero.Fs, deploymentManifestPath string, environmentNames []string

apis := api.NewAPIs().Filter(api.RetainByName(apiNames))

mani, manifestLoadError := manifest.LoadManifest(&manifest.ManifestLoaderContext{
mani, manifestLoadError := manifest.LoadManifest(&manifest.LoaderContext{
Fs: fs,
ManifestPath: deploymentManifestPath,
Environments: environmentNames,
Expand Down
4 changes: 2 additions & 2 deletions cmd/monaco/runner/completion/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func EnvironmentByArg0(_ *cobra.Command, args []string, _ string) ([]string, cob
}

func loadEnvironmentsFromManifest(manifestPath string) ([]string, cobra.ShellCompDirective) {
man, _ := manifest.LoadManifest(&manifest.ManifestLoaderContext{
man, _ := manifest.LoadManifest(&manifest.LoaderContext{
Fs: afero.NewOsFs(),
ManifestPath: manifestPath,
})
Expand All @@ -122,7 +122,7 @@ func loadEnvironmentsFromManifest(manifestPath string) ([]string, cobra.ShellCom
func ProjectsFromManifest(_ *cobra.Command, args []string, _ string) ([]string, cobra.ShellCompDirective) {

manifestPath := args[0]
mani, _ := manifest.LoadManifest(&manifest.ManifestLoaderContext{
mani, _ := manifest.LoadManifest(&manifest.LoaderContext{
Fs: afero.NewOsFs(),
ManifestPath: manifestPath,
})
Expand Down
18 changes: 11 additions & 7 deletions pkg/manifest/manifest_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ import (
"strings"
)

type ManifestLoaderContext struct {
Fs afero.Fs
// LoaderContext holds all information for [LoadManifest]
type LoaderContext struct {
// Fs holds the abstraction of the file system.
Fs afero.Fs

// ManifestPath holds the path from where the manifest should be loaded.
ManifestPath string

// Environments is a filter to what environments should be loaded.
Expand Down Expand Up @@ -97,7 +101,7 @@ func (e projectLoaderError) Error() string {
return fmt.Sprintf("%s:%s: %s", e.ManifestPath, e.Project, e.Reason)
}

func LoadManifest(context *ManifestLoaderContext) (Manifest, []error) {
func LoadManifest(context *LoaderContext) (Manifest, []error) {
log.Debug("Loading manifest %q. Restrictions: groups=%q, environments=%q", context.ManifestPath, context.Groups, context.Environments)

manifestYAML, err := readManifestYAML(context)
Expand Down Expand Up @@ -229,7 +233,7 @@ func parseOAuth(a oAuth) (OAuth, error) {
}, nil
}

func readManifestYAML(context *ManifestLoaderContext) (manifest, error) {
func readManifestYAML(context *LoaderContext) (manifest, error) {
manifestPath := filepath.Clean(context.ManifestPath)

if !files.IsYamlFileExtension(manifestPath) {
Expand Down Expand Up @@ -298,7 +302,7 @@ func validateManifestVersion(manifestVersion string) error {
return nil
}

func toEnvironments(context *ManifestLoaderContext, groups []group) (map[string]EnvironmentDefinition, []error) { // nolint:gocognit
func toEnvironments(context *LoaderContext, groups []group) (map[string]EnvironmentDefinition, []error) { // nolint:gocognit
var errors []error
environments := make(map[string]EnvironmentDefinition)

Expand Down Expand Up @@ -366,7 +370,7 @@ func toEnvironments(context *ManifestLoaderContext, groups []group) (map[string]
return environments, nil
}

func shouldSkipEnv(context *ManifestLoaderContext, group group, env environment) bool {
func shouldSkipEnv(context *LoaderContext, group group, env environment) bool {
// if nothing is restricted, everything is allowed
if len(context.Groups) == 0 && len(context.Environments) == 0 {
return false
Expand All @@ -383,7 +387,7 @@ func shouldSkipEnv(context *ManifestLoaderContext, group group, env environment)
return true
}

func parseEnvironment(context *ManifestLoaderContext, config environment, group string) (EnvironmentDefinition, []error) {
func parseEnvironment(context *LoaderContext, config environment, group string) (EnvironmentDefinition, []error) {
var errs []error

auth, err := parseAuth(config.Auth)
Expand Down
2 changes: 1 addition & 1 deletion pkg/manifest/manifest_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1658,7 +1658,7 @@ environmentGroups: [{name: b, environments: [{name: c, url: {value: d}, auth: {t
fs := afero.NewMemMapFs()
assert.NoError(t, afero.WriteFile(fs, "manifest.yaml", []byte(test.manifestContent), 0400))

mani, errs := LoadManifest(&ManifestLoaderContext{
mani, errs := LoadManifest(&LoaderContext{
Fs: fs,
ManifestPath: "manifest.yaml",
Groups: test.groups,
Expand Down

0 comments on commit 70089b3

Please sign in to comment.