Skip to content

Commit

Permalink
Code review changes
Browse files Browse the repository at this point in the history
Just call migrate in root and add logging around migration. Also make
migration check function private
  • Loading branch information
chriskim06 committed Jun 5, 2020
1 parent ea01855 commit b7689de
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 16 deletions.
10 changes: 2 additions & 8 deletions cmd/krew/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,8 @@ func preRun(cmd *cobra.Command, _ []string) error {
}

if _, ok := os.LookupEnv(constants.EnableMultiIndexSwitch); ok {
isMigrated, err = indexmigration.Done(paths)
if err != nil {
return errors.Wrap(err, "error getting file info")
}
if !isMigrated {
if err := indexmigration.Migrate(paths); err != nil {
return errors.Wrap(err, "failed to automatically migrate index")
}
if err := indexmigration.Migrate(paths); err != nil {
return errors.Wrap(err, "failed to automatically migrate index")
}
}

Expand Down
12 changes: 7 additions & 5 deletions internal/indexmigration/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import (
"sigs.k8s.io/krew/internal/environment"
)

// Done checks if the krew installation requires a migration to support multiple indexes.
// done checks if the krew installation requires a migration to support multiple indexes.
// A migration is necessary when the index directory contains a ".git" directory.
func Done(paths environment.Paths) (bool, error) {
func done(paths environment.Paths) (bool, error) {
_, err := os.Stat(filepath.Join(paths.IndexBase(), ".git"))
if err != nil && os.IsNotExist(err) {
return true, nil
Expand All @@ -36,14 +36,16 @@ func Done(paths environment.Paths) (bool, error) {

// Migrate removes the index directory and then clones krew-index to the new default index path.
func Migrate(paths environment.Paths) error {
isMigrated, err := Done(paths)
isMigrated, err := done(paths)
if err != nil {
return errors.Wrap(err, "failed to check if index migration is complete")
}
if isMigrated {
klog.V(2).Infoln("Already migrated")
klog.V(2).Infoln("already migrated")
return nil
}

klog.Info("migrating krew index layout")
indexPath := paths.IndexBase()
tmpPath := filepath.Join(paths.BasePath(), "tmp_index_migration")
newPath := filepath.Join(paths.IndexBase(), "default")
Expand All @@ -60,6 +62,6 @@ func Migrate(paths environment.Paths) error {
return errors.Wrapf(err, "could not move temporary index directory %q to new location %q", tmpPath, newPath)
}

klog.Infof("Migration completed successfully.")
klog.Info("migration completed successfully")
return nil
}
6 changes: 3 additions & 3 deletions internal/indexmigration/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestIsMigrated(t *testing.T) {
}

newPaths := environment.NewPaths(tmpDir.Root())
actual, err := Done(newPaths)
actual, err := done(newPaths)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -83,8 +83,8 @@ func TestMigrate(t *testing.T) {
if err != nil {
t.Fatal(err)
}
done, err := Done(newPaths)
if err != nil || !done {
migrationDone, err := done(newPaths)
if err != nil || !migrationDone {
t.Errorf("expected migration to be done: %s", err)
}
})
Expand Down

0 comments on commit b7689de

Please sign in to comment.