Merge lp:~sergiusens/snappy/dont_get_this into lp:~snappy-dev/snappy/snappy-moved-to-github

[Sergio Schvezov]

https://golang.org/doc/effective_go.html#Getters

Even though these aren't properties for fields per se, they are properties of the system and I'm going to use common idomatic go from around (no Get in the names).

[Michael Vogt (mvo)]

Thanks Sergio, this looks good and is more idiomatic go. The only small adjustment I made is that "OtherPart" -> "otherPart" and "CurrentPart" -> "currentPart" as I consider this more internal API (Installed() is the API the user should use).

review: Approve

=== modified file 'cmd/snappy/cmd_booted.go'
--- cmd/snappy/cmd_booted.go	2015-01-26 14:13:15 +0000
+++ cmd/snappy/cmd_booted.go	2015-01-27 19:16:59 +0000
@@ -15,7 +15,7 @@
 }
 
 func (x *CmdBooted) Execute(args []string) (err error) {
-	parts, err := snappy.GetInstalledSnappsByType("core")
+	parts, err := snappy.InstalledSnappsByType("core")
 	if err != nil {
 		return err
 	}
 
=== modified file 'cmd/snappy/cmd_info.go'
--- cmd/snappy/cmd_info.go	2015-01-26 14:30:14 +0000
+++ cmd/snappy/cmd_info.go	2015-01-27 19:16:59 +0000
@@ -25,13 +25,13 @@
 
 func info() error {
 	release := "unknown"
-	parts, err := snappy.GetInstalledSnappsByType("core")
+	parts, err := snappy.InstalledSnappsByType("core")
 	if len(parts) == 1 && err == nil {
 		release = parts[0].(*snappy.SystemImagePart).Channel()
 	}
 
-	frameworks, _ := snappy.GetInstalledSnappNamesByType("framework")
-	apps, _ := snappy.GetInstalledSnappNamesByType("app")
+	frameworks, _ := snappy.InstalledSnappNamesByType("framework")
+	apps, _ := snappy.InstalledSnappNamesByType("app")
 
 	fmt.Printf("release: %s\n", release)
 	fmt.Printf("architecture: %s\n", snappy.Architecture())
 
=== modified file 'snappy/list.go'
--- snappy/list.go	2015-01-26 14:13:15 +0000
+++ snappy/list.go	2015-01-27 19:16:59 +0000
@@ -3,11 +3,11 @@
 func ListInstalled() ([]Part, error) {
 	m := NewMetaRepository()
 
-	return m.GetInstalled()
+	return m.Installed()
 }
 
 func ListUpdates() ([]Part, error) {
 	m := NewMetaRepository()
 
-	return m.GetUpdates()
+	return m.Updates()
 }
 
=== modified file 'snappy/parts.go'
--- snappy/parts.go	2015-01-27 08:18:14 +0000
+++ snappy/parts.go	2015-01-27 19:16:59 +0000
@@ -42,8 +42,8 @@
 	Search(terms string) ([]Part, error)
 	Details(snappName string) ([]Part, error)
 
-	GetUpdates() ([]Part, error)
-	GetInstalled() ([]Part, error)
+	Updates() ([]Part, error)
+	Installed() ([]Part, error)
 }
 
 type MetaRepository struct {
@@ -70,9 +70,9 @@
 	return m
 }
 
-func (m *MetaRepository) GetInstalled() (parts []Part, err error) {
+func (m *MetaRepository) Installed() (parts []Part, err error) {
 	for _, r := range m.all {
-		installed, err := r.GetInstalled()
+		installed, err := r.Installed()
 		if err != nil {
 			return parts, err
 		}
@@ -82,9 +82,9 @@
 	return parts, err
 }
 
-func (m *MetaRepository) GetUpdates() (parts []Part, err error) {
+func (m *MetaRepository) Updates() (parts []Part, err error) {
 	for _, r := range m.all {
-		updates, err := r.GetUpdates()
+		updates, err := r.Updates()
 		if err != nil {
 			return parts, err
 		}
@@ -118,9 +118,9 @@
 	return parts, err
 }
 
-func GetInstalledSnappsByType(searchExp string) (res []Part, err error) {
+func InstalledSnappsByType(searchExp string) (res []Part, err error) {
 	m := NewMetaRepository()
-	installed, err := m.GetInstalled()
+	installed, err := m.Installed()
 	if err != nil {
 		return res, err
 	}
@@ -138,17 +138,17 @@
 	return
 }
 
-var GetInstalledSnappNamesByType = func(snappType string) (res []string, err error) {
-	installed, err := GetInstalledSnappsByType(snappType)
+var InstalledSnappNamesByType = func(snappType string) (res []string, err error) {
+	installed, err := InstalledSnappsByType(snappType)
 	for _, part := range installed {
 		res = append(res, part.Name())
 	}
 	return
 }
 
-func GetInstalledSnappByName(needle string) Part {
+func InstalledSnappByName(needle string) Part {
 	m := NewMetaRepository()
-	installed, err := m.GetInstalled()
+	installed, err := m.Installed()
 	if err != nil {
 		return nil
 	}
 
=== modified file 'snappy/remove.go'
--- snappy/remove.go	2015-01-26 14:13:15 +0000
+++ snappy/remove.go	2015-01-27 19:16:59 +0000
@@ -1,7 +1,7 @@
 package snappy
 
 func Remove(partName string) error {
-	part := GetInstalledSnappByName(partName)
+	part := InstalledSnappByName(partName)
 	if part != nil {
 		if err := part.Uninstall(); err != nil {
 			return err
 
=== modified file 'snappy/snapp.go'
--- snappy/snapp.go	2015-01-26 14:30:14 +0000
+++ snappy/snapp.go	2015-01-27 19:16:59 +0000
@@ -181,13 +181,11 @@
 	return versions, err
 }
 
-func (s *SnappLocalRepository) GetUpdates() (parts []Part, err error) {
-
+func (s *SnappLocalRepository) Updates() (parts []Part, err error) {
 	return parts, err
 }
 
-func (s *SnappLocalRepository) GetInstalled() (parts []Part, err error) {
-
+func (s *SnappLocalRepository) Installed() (parts []Part, err error) {
 	globExpr := path.Join(s.path, "*", "*", "meta", "package.yaml")
 	matches, err := filepath.Glob(globExpr)
 	if err != nil {
@@ -332,7 +330,7 @@
 
 	// set headers
 	req.Header.Set("Accept", "application/hal+json")
-	frameworks, _ := GetInstalledSnappNamesByType("framework")
+	frameworks, _ := InstalledSnappNamesByType("framework")
 	frameworks = append(frameworks, "ubuntu-core-15.04-dev1")
 	req.Header.Set("X-Ubuntu-Frameworks", strings.Join(frameworks, ","))
 	req.Header.Set("X-Ubuntu-Architecture", Architecture())
@@ -366,7 +364,7 @@
 
 	// set headers
 	req.Header.Set("Accept", "application/hal+json")
-	frameworks, _ := GetInstalledSnappNamesByType("framework")
+	frameworks, _ := InstalledSnappNamesByType("framework")
 	frameworks = append(frameworks, "ubuntu-core-15.04-dev1")
 	req.Header.Set("X-Ubuntu-Frameworks", strings.Join(frameworks, ","))
 	req.Header.Set("X-Ubuntu-Architecture", Architecture())
@@ -393,10 +391,10 @@
 	return parts, err
 }
 
-func (s *SnappUbuntuStoreRepository) GetUpdates() (parts []Part, err error) {
+func (s *SnappUbuntuStoreRepository) Updates() (parts []Part, err error) {
 	// the store only supports apps and framworks currently, so no
 	// sense in sending it our ubuntu-core snapp
-	installed, err := GetInstalledSnappNamesByType("app,framework")
+	installed, err := InstalledSnappNamesByType("app,framework")
 	if err != nil || len(installed) == 0 {
 		return parts, err
 	}
@@ -431,6 +429,6 @@
 	return parts, nil
 }
 
-func (s *SnappUbuntuStoreRepository) GetInstalled() (parts []Part, err error) {
+func (s *SnappUbuntuStoreRepository) Installed() (parts []Part, err error) {
 	return parts, err
 }
 
=== modified file 'snappy/snapp_test.go'
--- snappy/snapp_test.go	2015-01-23 19:28:17 +0000
+++ snappy/snapp_test.go	2015-01-27 19:16:59 +0000
@@ -108,7 +108,7 @@
 	snapp := NewLocalSnappRepository(path.Join(s.tempdir, "apps"))
 	c.Assert(snapp, NotNil)
 
-	installed, err := snapp.GetInstalled()
+	installed, err := snapp.Installed()
 	c.Assert(err, IsNil)
 	c.Assert(len(installed), Equals, 1)
 	c.Assert(installed[0].Name(), Equals, "hello-app")
@@ -159,16 +159,16 @@
 const MockUpdatesJson = `
 [
     {
-        "status": "Published", 
-        "name": "hello-world", 
-        "changelog": "", 
-        "icon_url": "https://myapps.developer.ubuntu.com/site_media/appmedia/2015/01/hello.svg.png", 
-        "title": "Hello world example", 
-        "binary_filesize": 31166, 
-        "anon_download_url": "https://public.apps.ubuntu.com/anon/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap", 
-        "allow_unauthenticated": true, 
-        "version": "1.0.5", 
-        "download_url": "https://public.apps.ubuntu.com/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap", 
+        "status": "Published",
+        "name": "hello-world",
+        "changelog": "",
+        "icon_url": "https://myapps.developer.ubuntu.com/site_media/appmedia/2015/01/hello.svg.png",
+        "title": "Hello world example",
+        "binary_filesize": 31166,
+        "anon_download_url": "https://public.apps.ubuntu.com/anon/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap",
+        "allow_unauthenticated": true,
+        "version": "1.0.5",
+        "download_url": "https://public.apps.ubuntu.com/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap",
         "download_sha512": "3e8b192e18907d8195c2e380edd048870eda4f6dbcba8f65e4625d6efac3c37d11d607147568ade6f002b6baa30762c6da02e7ee462de7c56301ddbdc10d87f6"
     }
 ]
@@ -270,17 +270,17 @@
 	c.Assert(results[0].Description(), Equals, "Show random XKCD comic")
 }
 
-func mockGetInstalledSnappNamesByType(mockSnapps []string) (mockRestorer func()) {
-	origFunc := GetInstalledSnappNamesByType
-	GetInstalledSnappNamesByType = func(snappType string) (res []string, err error) {
+func mockInstalledSnappNamesByType(mockSnapps []string) (mockRestorer func()) {
+	origFunc := InstalledSnappNamesByType
+	InstalledSnappNamesByType = func(snappType string) (res []string, err error) {
 		return mockSnapps, nil
 	}
 	return func() {
-		GetInstalledSnappNamesByType = origFunc
+		InstalledSnappNamesByType = origFunc
 	}
 }
 
-func (s *SnappTestSuite) TestUbuntuStoreRepositoryGetUpdates(c *C) {
+func (s *SnappTestSuite) TestUbuntuStoreRepositoryUpdates(c *C) {
 	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		json_req, err := ioutil.ReadAll(r.Body)
 		c.Assert(err, IsNil)
@@ -295,20 +295,20 @@
 	c.Assert(snapp, NotNil)
 	snapp.bulkUri = mockServer.URL + "/updates/"
 
-	// override the real GetInstalledSnappNamesByType to return our
+	// override the real InstalledSnappNamesByType to return our
 	// mock data
-	mockRestorer := mockGetInstalledSnappNamesByType([]string{"hello-world"})
+	mockRestorer := mockInstalledSnappNamesByType([]string{"hello-world"})
 	defer mockRestorer()
 
 	// the actual test
-	results, err := snapp.GetUpdates()
+	results, err := snapp.Updates()
 	c.Assert(err, IsNil)
 	c.Assert(len(results), Equals, 1)
 	c.Assert(results[0].Name(), Equals, "hello-world")
 	c.Assert(results[0].Version(), Equals, "1.0.5")
 }
 
-func (s *SnappTestSuite) TestUbuntuStoreRepositoryGetUpdatesNoSnapps(c *C) {
+func (s *SnappTestSuite) TestUbuntuStoreRepositoryUpdatesNoSnapps(c *C) {
 
 	snapp := NewUbuntuStoreSnappRepository()
 	c.Assert(snapp, NotNil)
@@ -316,16 +316,16 @@
 	// ensure we do not hit the net if there is nothing installed
 	// (otherwise the store will send us all snapps)
 	snapp.bulkUri = "http://i-do.not-exist.really-not"
-	mockRestorer := mockGetInstalledSnappNamesByType([]string{})
+	mockRestorer := mockInstalledSnappNamesByType([]string{})
 	defer mockRestorer()
 
 	// the actual test
-	results, err := snapp.GetUpdates()
+	results, err := snapp.Updates()
 	c.Assert(err, IsNil)
 	c.Assert(len(results), Equals, 0)
 }
 
-func (s *SnappTestSuite) TestUbuntuStoreRepositoryGetDetails(c *C) {
+func (s *SnappTestSuite) TestUbuntuStoreRepositoryDetails(c *C) {
 	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		c.Assert(strings.HasSuffix(r.URL.String(), "xkcd-webserver"), Equals, true)
 		io.WriteString(w, MockDetailsJson)
 
=== modified file 'snappy/systemimage.go'
--- snappy/systemimage.go	2015-01-27 11:41:12 +0000
+++ snappy/systemimage.go	2015-01-27 19:16:59 +0000
@@ -416,7 +416,7 @@
 		partition:      s.partition}, err
 }
 
-func (s *SystemImageRepository) getCurrentPart() Part {
+func (s *SystemImageRepository) CurrentPart() Part {
 	configFile := s.myroot + systemImageChannelConfig
 	part, err := s.makePartFromSystemImageConfigFile(configFile, true)
 	if err != nil {
@@ -426,7 +426,7 @@
 }
 
 // Returns the part associated with the other rootfs (if any)
-func (s *SystemImageRepository) getOtherPart() Part {
+func (s *SystemImageRepository) OtherPart() Part {
 	var part Part
 	s.partition.RunWithOther(func(otherRoot string) (err error) {
 		configFile := s.myroot + otherRoot + systemImageChannelConfig
@@ -442,7 +442,7 @@
 func (s *SystemImageRepository) Search(terms string) (versions []Part, err error) {
 	if strings.Contains(terms, systemImagePartName) {
 		s.proxy.Information()
-		part := s.getCurrentPart()
+		part := s.CurrentPart()
 		versions = append(versions, part)
 	}
 	return versions, err
@@ -451,14 +451,13 @@
 func (s *SystemImageRepository) Details(snappName string) (versions []Part, err error) {
 	if snappName == systemImagePartName {
 		s.proxy.Information()
-		part := s.getCurrentPart()
+		part := s.CurrentPart()
 		versions = append(versions, part)
 	}
 	return versions, err
 }
 
-func (s *SystemImageRepository) GetUpdates() (parts []Part, err error) {
-
+func (s *SystemImageRepository) Updates() (parts []Part, err error) {
 	if _, err = s.proxy.CheckForUpdate(); err != nil {
 		return parts, err
 	}
@@ -479,15 +478,15 @@
 	return parts, err
 }
 
-func (s *SystemImageRepository) GetInstalled() (parts []Part, err error) {
+func (s *SystemImageRepository) Installed() (parts []Part, err error) {
 	// current partition
-	curr := s.getCurrentPart()
+	curr := s.CurrentPart()
 	if curr != nil {
 		parts = append(parts, curr)
 	}
 
 	// other partition
-	other := s.getOtherPart()
+	other := s.OtherPart()
 	if other != nil {
 		parts = append(parts, other)
 	}
 
=== modified file 'snappy/systemimage_test.go'
--- snappy/systemimage_test.go	2015-01-26 10:05:08 +0000
+++ snappy/systemimage_test.go	2015-01-27 19:16:59 +0000
@@ -248,7 +248,7 @@
 
 func (s *SITestSuite) TestTestInstalled(c *C) {
 	// whats installed
-	parts, err := s.systemImage.GetInstalled()
+	parts, err := s.systemImage.Installed()
 	c.Assert(err, IsNil)
 	// we have one active and one inactive
 	c.Assert(len(parts), Equals, 2)
@@ -262,16 +262,16 @@
 	c.Assert(parts[1].Version(), Equals, "3.14")
 }
 
-func (s *SITestSuite) TestGetUpdateNoUpdate(c *C) {
-	parts, err := s.systemImage.GetUpdates()
+func (s *SITestSuite) TestUpdateNoUpdate(c *C) {
+	parts, err := s.systemImage.Updates()
 	c.Assert(err, IsNil)
 	c.Assert(len(parts), Equals, 0)
 }
 
-func (s *SITestSuite) TestGetUpdateHasUpdate(c *C) {
+func (s *SITestSuite) TestUpdateHasUpdate(c *C) {
 	// add a update
 	s.mockSystemImage.info["target_build_number"] = "3.14"
-	parts, err := s.systemImage.GetUpdates()
+	parts, err := s.systemImage.Updates()
 	c.Assert(err, IsNil)
 	c.Assert(len(parts), Equals, 1)
 	c.Assert(parts[0].Name(), Equals, "ubuntu-core")
@@ -308,7 +308,7 @@
 func (s *SITestSuite) TestSystemImagePartInstallUpdatesPartition(c *C) {
 	// add a update
 	s.mockSystemImage.info["target_build_number"] = "3.14"
-	parts, err := s.systemImage.GetUpdates()
+	parts, err := s.systemImage.Updates()
 
 	sp := parts[0].(*SystemImagePart)
 	mockPartition := MockPartition{}
@@ -322,7 +322,7 @@
 func (s *SITestSuite) TestSystemImagePartInstall(c *C) {
 	// add a update
 	s.mockSystemImage.info["target_build_number"] = "3.14"
-	parts, err := s.systemImage.GetUpdates()
+	parts, err := s.systemImage.Updates()
 
 	sp := parts[0].(*SystemImagePart)
 	mockPartition := MockPartition{}