Repository: kubernetes-sigs/krew
Pull request #275 (2019-07-17 22:13:20)

minor: rename environment.Path receipt path methods
Purely method rename patch.

- InstallReceiptPath --> InstallReceiptsPath: indicating the path is containng
  multiple receipts.
- PluginReceiptPath --> PluginInstallReceiptPath: making it clear to answer
  "what receipt?" (A: install receipt), similarly consistent with the method
  name above, just prefixed+singular.

/assign @corneliusweig

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/275#" title="Author self-approved">ahmetb</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fkrew).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/OWNERS)~~ [ahmetb]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
corneliusweig: /lgtm
/approve
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/krew/pull/275#" title="Author self-approved">ahmetb</a>*, *<a href="https://github.com/kubernetes-sigs/krew/pull/275#issuecomment-512600399" title="Approved">corneliusweig</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fkrew).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/krew/blob/master/OWNERS)~~ [ahmetb,corneliusweig]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/cmd/krew/cmd/root.go b/cmd/krew/cmd/root.go
index ecac91f..93856a1 100644
--- a/cmd/krew/cmd/root.go
+++ b/cmd/krew/cmd/root.go
@@ -82,7 +82,7 @@ func init() {
 		paths.DownloadPath(),
 		paths.InstallPath(),
 		paths.BinPath(),
-		paths.InstallReceiptPath()); err != nil {
+		paths.InstallReceiptsPath()); err != nil {
 		glog.Fatal(err)
 	}
 }
diff --git a/pkg/environment/environment.go b/pkg/environment/environment.go
index b2d9ffc..a8925b3 100644
--- a/pkg/environment/environment.go
+++ b/pkg/environment/environment.go
@@ -65,10 +65,10 @@ func (p Paths) IndexPath() string { return filepath.Join(p.base, "index") }
 // e.g. {BasePath}/index/plugins/
 func (p Paths) IndexPluginsPath() string { return filepath.Join(p.base, "index", "plugins") }
 
-// InstallReceiptPath returns the base directory where plugin receipts are stored.
+// InstallReceiptsPath returns the base directory where plugin receipts are stored.
 //
 // e.g. {BasePath}/receipts
-func (p Paths) InstallReceiptPath() string { return filepath.Join(p.base, "receipts") }
+func (p Paths) InstallReceiptsPath() string { return filepath.Join(p.base, "receipts") }
 
 // BinPath returns the path where plugin executable symbolic links are found.
 // This path should be added to $PATH in client machine.
@@ -92,11 +92,11 @@ func (p Paths) PluginInstallPath(plugin string) string {
 	return filepath.Join(p.InstallPath(), plugin)
 }
 
-// PluginReceiptPath returns the path to the install receipt for plugin.
+// PluginInstallReceiptPath returns the path to the install receipt for plugin.
 //
-// e.g. {InstallReceiptPath}/{plugin}.yaml
-func (p Paths) PluginReceiptPath(plugin string) string {
-	return filepath.Join(p.InstallReceiptPath(), plugin+constants.ManifestExtension)
+// e.g. {InstallReceiptsPath}/{plugin}.yaml
+func (p Paths) PluginInstallReceiptPath(plugin string) string {
+	return filepath.Join(p.InstallReceiptsPath(), plugin+constants.ManifestExtension)
 }
 
 // PluginVersionInstallPath returns the path to the specified version of specified
diff --git a/pkg/info/info.go b/pkg/info/info.go
index e9c0f08..6f3cd60 100644
--- a/pkg/info/info.go
+++ b/pkg/info/info.go
@@ -28,7 +28,7 @@ import (
 // LoadManifestFromReceiptOrIndex tries to load a plugin manifest from the
 // receipts directory or from the index directory if the former fails.
 func LoadManifestFromReceiptOrIndex(p environment.Paths, name string) (index.Plugin, error) {
-	receipt, err := indexscanner.LoadPluginFileFromFS(p.InstallReceiptPath(), name)
+	receipt, err := indexscanner.LoadPluginFileFromFS(p.InstallReceiptsPath(), name)
 
 	if err == nil {
 		glog.V(3).Infof("Found plugin manifest for %q in the receipts dir", name)
diff --git a/pkg/installation/install.go b/pkg/installation/install.go
index 5a3d2a3..700cd8e 100644
--- a/pkg/installation/install.go
+++ b/pkg/installation/install.go
@@ -85,7 +85,7 @@ func Install(p environment.Paths, plugin index.Plugin, forceDownloadFile string)
 		return errors.Wrap(err, "install failed")
 	}
 	glog.V(3).Infof("Storing install receipt for plugin %s", plugin.Name)
-	err = receipt.Store(plugin, p.PluginReceiptPath(plugin.Name))
+	err = receipt.Store(plugin, p.PluginInstallReceiptPath(plugin.Name))
 	return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail")
 }
 
@@ -137,10 +137,10 @@ func Uninstall(p environment.Paths, name string) error {
 	if err := os.RemoveAll(pluginInstallPath); err != nil {
 		return errors.Wrapf(err, "could not remove plugin directory %q", pluginInstallPath)
 	}
-	pluginReceiptPath := p.PluginReceiptPath(name)
-	glog.V(3).Infof("Deleting plugin receipt %q", pluginReceiptPath)
-	err = os.Remove(pluginReceiptPath)
-	return errors.Wrapf(err, "could not remove plugin receipt %q", pluginReceiptPath)
+	PluginInstallReceiptPath := p.PluginInstallReceiptPath(name)
+	glog.V(3).Infof("Deleting plugin receipt %q", PluginInstallReceiptPath)
+	err = os.Remove(PluginInstallReceiptPath)
+	return errors.Wrapf(err, "could not remove plugin receipt %q", PluginInstallReceiptPath)
 }
 
 func createOrUpdateLink(binDir string, binary string, plugin string) error {
diff --git a/pkg/installation/upgrade.go b/pkg/installation/upgrade.go
index 75d2e79..cd4b8cb 100644
--- a/pkg/installation/upgrade.go
+++ b/pkg/installation/upgrade.go
@@ -47,7 +47,7 @@ func Upgrade(p environment.Paths, plugin index.Plugin) error {
 	}
 
 	glog.V(2).Infof("Upgrading install receipt for plugin %s", plugin.Name)
-	if err = receipt.Store(plugin, p.PluginReceiptPath(plugin.Name)); err != nil {
+	if err = receipt.Store(plugin, p.PluginInstallReceiptPath(plugin.Name)); err != nil {
 		return errors.Wrap(err, "installation receipt could not be stored, uninstall may fail")
 	}
 
diff --git a/pkg/receiptsmigration/migration.go b/pkg/receiptsmigration/migration.go
index 9dfb9b9..1008fae 100644
--- a/pkg/receiptsmigration/migration.go
+++ b/pkg/receiptsmigration/migration.go
@@ -40,7 +40,7 @@ const (
 // Done checks if the krew installation requires a migration.
 // It considers a migration necessary when plugins are installed, but no receipts are present.
 func Done(newPaths environment.Paths) (bool, error) {
-	receipts, err := ioutil.ReadDir(newPaths.InstallReceiptPath())
+	receipts, err := ioutil.ReadDir(newPaths.InstallReceiptsPath())
 	if err != nil {
 		return false, err
 	}
@@ -83,7 +83,7 @@ func Migrate(newPaths environment.Paths) error {
 	glog.Infoln("These plugins will be reinstalled: ", installed)
 
 	// krew must be skipped by the normal migration logic
-	if err := copyKrewManifest(newPaths.IndexPluginsPath(), newPaths.InstallReceiptPath()); err != nil {
+	if err := copyKrewManifest(newPaths.IndexPluginsPath(), newPaths.InstallReceiptsPath()); err != nil {
 		return errors.Wrapf(err, "failed to copy krew manifest")
 	}
 


Test patch:
diff --git a/pkg/environment/environment_test.go b/pkg/environment/environment_test.go
index c54a240..21b7e15 100644
--- a/pkg/environment/environment_test.go
+++ b/pkg/environment/environment_test.go
@@ -72,11 +72,11 @@ func TestPaths(t *testing.T) {
 	if got := p.DownloadPath(); !strings.HasSuffix(got, "krew-downloads") {
 		t.Fatalf("DownloadPath()=%s; expected suffix 'krew-downloads'", got)
 	}
-	if got := p.InstallReceiptPath(); !strings.HasSuffix(got, filepath.FromSlash("receipts")) {
-		t.Fatalf("InstallReceiptPath()=%s; expected suffix 'receipts'", got)
+	if got := p.InstallReceiptsPath(); !strings.HasSuffix(got, filepath.FromSlash("receipts")) {
+		t.Fatalf("InstallReceiptsPath()=%s; expected suffix 'receipts'", got)
 	}
-	if got := p.PluginReceiptPath("my-plugin"); !strings.HasSuffix(got, filepath.FromSlash("receipts/my-plugin.yaml")) {
-		t.Fatalf("PluginReceiptPath()=%s; expected suffix 'receipts/my-plugin.yaml'", got)
+	if got := p.PluginInstallReceiptPath("my-plugin"); !strings.HasSuffix(got, filepath.FromSlash("receipts/my-plugin.yaml")) {
+		t.Fatalf("PluginInstallReceiptPath()=%s; expected suffix 'receipts/my-plugin.yaml'", got)
 	}
 }
 
diff --git a/pkg/info/info_test.go b/pkg/info/info_test.go
index 38c1d15..829c7b6 100644
--- a/pkg/info/info_test.go
+++ b/pkg/info/info_test.go
@@ -69,7 +69,7 @@ func TestLoadManifestFromReceiptOrIndex(t *testing.T) {
 		{
 			name: "manifest in receipts",
 			prepare: func(paths environment.Paths, tmpDir *testutil.TempDir) {
-				path := paths.PluginReceiptPath(pluginName)
+				path := paths.PluginInstallReceiptPath(pluginName)
 				tmpDir.Write(path, yamlBytes)
 			},
 		},
@@ -83,7 +83,7 @@ func TestLoadManifestFromReceiptOrIndex(t *testing.T) {
 		{
 			name: "invalid manifest in receipts",
 			prepare: func(paths environment.Paths, tmpDir *testutil.TempDir) {
-				path := paths.PluginReceiptPath(pluginName)
+				path := paths.PluginInstallReceiptPath(pluginName)
 				tmpDir.Write(path, []byte("invalid yaml file"))
 			},
 			shouldErr: true,