Snappy

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

  1. dont_get_this
  2. Merge into snappy-moved-to-github
Proposed by Sergio Schvezov
Status: Merged
Merged at revision: 123
Proposed branch: lp:~sergiusens/snappy/dont_get_this
Merge into: lp:~snappy-dev/snappy/snappy-moved-to-github
Diff against target: 423 lines (+64/-67)
9 files modified
cmd/snappy/cmd_booted.go (+1/-1)
cmd/snappy/cmd_info.go (+3/-3)
snappy/list.go (+2/-2)
snappy/parts.go (+12/-12)
snappy/remove.go (+1/-1)
snappy/snapp.go (+7/-9)
snappy/snapp_test.go (+23/-23)
snappy/systemimage.go (+8/-9)
snappy/systemimage_test.go (+7/-7)
To merge this branch: bzr merge lp:~sergiusens/snappy/dont_get_this
Reviewer Review Type Date Requested Status
Michael Vogt (community) Approve
Review via email: mp+247765@code.launchpad.net

Description of the change

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).

To post a comment you must log in.
Revision history for this message
Michael Vogt (mvo) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmd/snappy/cmd_booted.go'
2--- cmd/snappy/cmd_booted.go 2015-01-26 14:13:15 +0000
3+++ cmd/snappy/cmd_booted.go 2015-01-27 19:16:59 +0000
4@@ -15,7 +15,7 @@
5 }
6
7 func (x *CmdBooted) Execute(args []string) (err error) {
8- parts, err := snappy.GetInstalledSnappsByType("core")
9+ parts, err := snappy.InstalledSnappsByType("core")
10 if err != nil {
11 return err
12 }
13
14=== modified file 'cmd/snappy/cmd_info.go'
15--- cmd/snappy/cmd_info.go 2015-01-26 14:30:14 +0000
16+++ cmd/snappy/cmd_info.go 2015-01-27 19:16:59 +0000
17@@ -25,13 +25,13 @@
18
19 func info() error {
20 release := "unknown"
21- parts, err := snappy.GetInstalledSnappsByType("core")
22+ parts, err := snappy.InstalledSnappsByType("core")
23 if len(parts) == 1 && err == nil {
24 release = parts[0].(*snappy.SystemImagePart).Channel()
25 }
26
27- frameworks, _ := snappy.GetInstalledSnappNamesByType("framework")
28- apps, _ := snappy.GetInstalledSnappNamesByType("app")
29+ frameworks, _ := snappy.InstalledSnappNamesByType("framework")
30+ apps, _ := snappy.InstalledSnappNamesByType("app")
31
32 fmt.Printf("release: %s\n", release)
33 fmt.Printf("architecture: %s\n", snappy.Architecture())
34
35=== modified file 'snappy/list.go'
36--- snappy/list.go 2015-01-26 14:13:15 +0000
37+++ snappy/list.go 2015-01-27 19:16:59 +0000
38@@ -3,11 +3,11 @@
39 func ListInstalled() ([]Part, error) {
40 m := NewMetaRepository()
41
42- return m.GetInstalled()
43+ return m.Installed()
44 }
45
46 func ListUpdates() ([]Part, error) {
47 m := NewMetaRepository()
48
49- return m.GetUpdates()
50+ return m.Updates()
51 }
52
53=== modified file 'snappy/parts.go'
54--- snappy/parts.go 2015-01-27 08:18:14 +0000
55+++ snappy/parts.go 2015-01-27 19:16:59 +0000
56@@ -42,8 +42,8 @@
57 Search(terms string) ([]Part, error)
58 Details(snappName string) ([]Part, error)
59
60- GetUpdates() ([]Part, error)
61- GetInstalled() ([]Part, error)
62+ Updates() ([]Part, error)
63+ Installed() ([]Part, error)
64 }
65
66 type MetaRepository struct {
67@@ -70,9 +70,9 @@
68 return m
69 }
70
71-func (m *MetaRepository) GetInstalled() (parts []Part, err error) {
72+func (m *MetaRepository) Installed() (parts []Part, err error) {
73 for _, r := range m.all {
74- installed, err := r.GetInstalled()
75+ installed, err := r.Installed()
76 if err != nil {
77 return parts, err
78 }
79@@ -82,9 +82,9 @@
80 return parts, err
81 }
82
83-func (m *MetaRepository) GetUpdates() (parts []Part, err error) {
84+func (m *MetaRepository) Updates() (parts []Part, err error) {
85 for _, r := range m.all {
86- updates, err := r.GetUpdates()
87+ updates, err := r.Updates()
88 if err != nil {
89 return parts, err
90 }
91@@ -118,9 +118,9 @@
92 return parts, err
93 }
94
95-func GetInstalledSnappsByType(searchExp string) (res []Part, err error) {
96+func InstalledSnappsByType(searchExp string) (res []Part, err error) {
97 m := NewMetaRepository()
98- installed, err := m.GetInstalled()
99+ installed, err := m.Installed()
100 if err != nil {
101 return res, err
102 }
103@@ -138,17 +138,17 @@
104 return
105 }
106
107-var GetInstalledSnappNamesByType = func(snappType string) (res []string, err error) {
108- installed, err := GetInstalledSnappsByType(snappType)
109+var InstalledSnappNamesByType = func(snappType string) (res []string, err error) {
110+ installed, err := InstalledSnappsByType(snappType)
111 for _, part := range installed {
112 res = append(res, part.Name())
113 }
114 return
115 }
116
117-func GetInstalledSnappByName(needle string) Part {
118+func InstalledSnappByName(needle string) Part {
119 m := NewMetaRepository()
120- installed, err := m.GetInstalled()
121+ installed, err := m.Installed()
122 if err != nil {
123 return nil
124 }
125
126=== modified file 'snappy/remove.go'
127--- snappy/remove.go 2015-01-26 14:13:15 +0000
128+++ snappy/remove.go 2015-01-27 19:16:59 +0000
129@@ -1,7 +1,7 @@
130 package snappy
131
132 func Remove(partName string) error {
133- part := GetInstalledSnappByName(partName)
134+ part := InstalledSnappByName(partName)
135 if part != nil {
136 if err := part.Uninstall(); err != nil {
137 return err
138
139=== modified file 'snappy/snapp.go'
140--- snappy/snapp.go 2015-01-26 14:30:14 +0000
141+++ snappy/snapp.go 2015-01-27 19:16:59 +0000
142@@ -181,13 +181,11 @@
143 return versions, err
144 }
145
146-func (s *SnappLocalRepository) GetUpdates() (parts []Part, err error) {
147-
148+func (s *SnappLocalRepository) Updates() (parts []Part, err error) {
149 return parts, err
150 }
151
152-func (s *SnappLocalRepository) GetInstalled() (parts []Part, err error) {
153-
154+func (s *SnappLocalRepository) Installed() (parts []Part, err error) {
155 globExpr := path.Join(s.path, "*", "*", "meta", "package.yaml")
156 matches, err := filepath.Glob(globExpr)
157 if err != nil {
158@@ -332,7 +330,7 @@
159
160 // set headers
161 req.Header.Set("Accept", "application/hal+json")
162- frameworks, _ := GetInstalledSnappNamesByType("framework")
163+ frameworks, _ := InstalledSnappNamesByType("framework")
164 frameworks = append(frameworks, "ubuntu-core-15.04-dev1")
165 req.Header.Set("X-Ubuntu-Frameworks", strings.Join(frameworks, ","))
166 req.Header.Set("X-Ubuntu-Architecture", Architecture())
167@@ -366,7 +364,7 @@
168
169 // set headers
170 req.Header.Set("Accept", "application/hal+json")
171- frameworks, _ := GetInstalledSnappNamesByType("framework")
172+ frameworks, _ := InstalledSnappNamesByType("framework")
173 frameworks = append(frameworks, "ubuntu-core-15.04-dev1")
174 req.Header.Set("X-Ubuntu-Frameworks", strings.Join(frameworks, ","))
175 req.Header.Set("X-Ubuntu-Architecture", Architecture())
176@@ -393,10 +391,10 @@
177 return parts, err
178 }
179
180-func (s *SnappUbuntuStoreRepository) GetUpdates() (parts []Part, err error) {
181+func (s *SnappUbuntuStoreRepository) Updates() (parts []Part, err error) {
182 // the store only supports apps and framworks currently, so no
183 // sense in sending it our ubuntu-core snapp
184- installed, err := GetInstalledSnappNamesByType("app,framework")
185+ installed, err := InstalledSnappNamesByType("app,framework")
186 if err != nil || len(installed) == 0 {
187 return parts, err
188 }
189@@ -431,6 +429,6 @@
190 return parts, nil
191 }
192
193-func (s *SnappUbuntuStoreRepository) GetInstalled() (parts []Part, err error) {
194+func (s *SnappUbuntuStoreRepository) Installed() (parts []Part, err error) {
195 return parts, err
196 }
197
198=== modified file 'snappy/snapp_test.go'
199--- snappy/snapp_test.go 2015-01-23 19:28:17 +0000
200+++ snappy/snapp_test.go 2015-01-27 19:16:59 +0000
201@@ -108,7 +108,7 @@
202 snapp := NewLocalSnappRepository(path.Join(s.tempdir, "apps"))
203 c.Assert(snapp, NotNil)
204
205- installed, err := snapp.GetInstalled()
206+ installed, err := snapp.Installed()
207 c.Assert(err, IsNil)
208 c.Assert(len(installed), Equals, 1)
209 c.Assert(installed[0].Name(), Equals, "hello-app")
210@@ -159,16 +159,16 @@
211 const MockUpdatesJson = `
212 [
213 {
214- "status": "Published",
215- "name": "hello-world",
216- "changelog": "",
217- "icon_url": "https://myapps.developer.ubuntu.com/site_media/appmedia/2015/01/hello.svg.png",
218- "title": "Hello world example",
219- "binary_filesize": 31166,
220- "anon_download_url": "https://public.apps.ubuntu.com/anon/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap",
221- "allow_unauthenticated": true,
222- "version": "1.0.5",
223- "download_url": "https://public.apps.ubuntu.com/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap",
224+ "status": "Published",
225+ "name": "hello-world",
226+ "changelog": "",
227+ "icon_url": "https://myapps.developer.ubuntu.com/site_media/appmedia/2015/01/hello.svg.png",
228+ "title": "Hello world example",
229+ "binary_filesize": 31166,
230+ "anon_download_url": "https://public.apps.ubuntu.com/anon/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap",
231+ "allow_unauthenticated": true,
232+ "version": "1.0.5",
233+ "download_url": "https://public.apps.ubuntu.com/download/com.ubuntu.snappy/hello-world/hello-world_1.0.5_all.snap",
234 "download_sha512": "3e8b192e18907d8195c2e380edd048870eda4f6dbcba8f65e4625d6efac3c37d11d607147568ade6f002b6baa30762c6da02e7ee462de7c56301ddbdc10d87f6"
235 }
236 ]
237@@ -270,17 +270,17 @@
238 c.Assert(results[0].Description(), Equals, "Show random XKCD comic")
239 }
240
241-func mockGetInstalledSnappNamesByType(mockSnapps []string) (mockRestorer func()) {
242- origFunc := GetInstalledSnappNamesByType
243- GetInstalledSnappNamesByType = func(snappType string) (res []string, err error) {
244+func mockInstalledSnappNamesByType(mockSnapps []string) (mockRestorer func()) {
245+ origFunc := InstalledSnappNamesByType
246+ InstalledSnappNamesByType = func(snappType string) (res []string, err error) {
247 return mockSnapps, nil
248 }
249 return func() {
250- GetInstalledSnappNamesByType = origFunc
251+ InstalledSnappNamesByType = origFunc
252 }
253 }
254
255-func (s *SnappTestSuite) TestUbuntuStoreRepositoryGetUpdates(c *C) {
256+func (s *SnappTestSuite) TestUbuntuStoreRepositoryUpdates(c *C) {
257 mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
258 json_req, err := ioutil.ReadAll(r.Body)
259 c.Assert(err, IsNil)
260@@ -295,20 +295,20 @@
261 c.Assert(snapp, NotNil)
262 snapp.bulkUri = mockServer.URL + "/updates/"
263
264- // override the real GetInstalledSnappNamesByType to return our
265+ // override the real InstalledSnappNamesByType to return our
266 // mock data
267- mockRestorer := mockGetInstalledSnappNamesByType([]string{"hello-world"})
268+ mockRestorer := mockInstalledSnappNamesByType([]string{"hello-world"})
269 defer mockRestorer()
270
271 // the actual test
272- results, err := snapp.GetUpdates()
273+ results, err := snapp.Updates()
274 c.Assert(err, IsNil)
275 c.Assert(len(results), Equals, 1)
276 c.Assert(results[0].Name(), Equals, "hello-world")
277 c.Assert(results[0].Version(), Equals, "1.0.5")
278 }
279
280-func (s *SnappTestSuite) TestUbuntuStoreRepositoryGetUpdatesNoSnapps(c *C) {
281+func (s *SnappTestSuite) TestUbuntuStoreRepositoryUpdatesNoSnapps(c *C) {
282
283 snapp := NewUbuntuStoreSnappRepository()
284 c.Assert(snapp, NotNil)
285@@ -316,16 +316,16 @@
286 // ensure we do not hit the net if there is nothing installed
287 // (otherwise the store will send us all snapps)
288 snapp.bulkUri = "http://i-do.not-exist.really-not"
289- mockRestorer := mockGetInstalledSnappNamesByType([]string{})
290+ mockRestorer := mockInstalledSnappNamesByType([]string{})
291 defer mockRestorer()
292
293 // the actual test
294- results, err := snapp.GetUpdates()
295+ results, err := snapp.Updates()
296 c.Assert(err, IsNil)
297 c.Assert(len(results), Equals, 0)
298 }
299
300-func (s *SnappTestSuite) TestUbuntuStoreRepositoryGetDetails(c *C) {
301+func (s *SnappTestSuite) TestUbuntuStoreRepositoryDetails(c *C) {
302 mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
303 c.Assert(strings.HasSuffix(r.URL.String(), "xkcd-webserver"), Equals, true)
304 io.WriteString(w, MockDetailsJson)
305
306=== modified file 'snappy/systemimage.go'
307--- snappy/systemimage.go 2015-01-27 11:41:12 +0000
308+++ snappy/systemimage.go 2015-01-27 19:16:59 +0000
309@@ -416,7 +416,7 @@
310 partition: s.partition}, err
311 }
312
313-func (s *SystemImageRepository) getCurrentPart() Part {
314+func (s *SystemImageRepository) CurrentPart() Part {
315 configFile := s.myroot + systemImageChannelConfig
316 part, err := s.makePartFromSystemImageConfigFile(configFile, true)
317 if err != nil {
318@@ -426,7 +426,7 @@
319 }
320
321 // Returns the part associated with the other rootfs (if any)
322-func (s *SystemImageRepository) getOtherPart() Part {
323+func (s *SystemImageRepository) OtherPart() Part {
324 var part Part
325 s.partition.RunWithOther(func(otherRoot string) (err error) {
326 configFile := s.myroot + otherRoot + systemImageChannelConfig
327@@ -442,7 +442,7 @@
328 func (s *SystemImageRepository) Search(terms string) (versions []Part, err error) {
329 if strings.Contains(terms, systemImagePartName) {
330 s.proxy.Information()
331- part := s.getCurrentPart()
332+ part := s.CurrentPart()
333 versions = append(versions, part)
334 }
335 return versions, err
336@@ -451,14 +451,13 @@
337 func (s *SystemImageRepository) Details(snappName string) (versions []Part, err error) {
338 if snappName == systemImagePartName {
339 s.proxy.Information()
340- part := s.getCurrentPart()
341+ part := s.CurrentPart()
342 versions = append(versions, part)
343 }
344 return versions, err
345 }
346
347-func (s *SystemImageRepository) GetUpdates() (parts []Part, err error) {
348-
349+func (s *SystemImageRepository) Updates() (parts []Part, err error) {
350 if _, err = s.proxy.CheckForUpdate(); err != nil {
351 return parts, err
352 }
353@@ -479,15 +478,15 @@
354 return parts, err
355 }
356
357-func (s *SystemImageRepository) GetInstalled() (parts []Part, err error) {
358+func (s *SystemImageRepository) Installed() (parts []Part, err error) {
359 // current partition
360- curr := s.getCurrentPart()
361+ curr := s.CurrentPart()
362 if curr != nil {
363 parts = append(parts, curr)
364 }
365
366 // other partition
367- other := s.getOtherPart()
368+ other := s.OtherPart()
369 if other != nil {
370 parts = append(parts, other)
371 }
372
373=== modified file 'snappy/systemimage_test.go'
374--- snappy/systemimage_test.go 2015-01-26 10:05:08 +0000
375+++ snappy/systemimage_test.go 2015-01-27 19:16:59 +0000
376@@ -248,7 +248,7 @@
377
378 func (s *SITestSuite) TestTestInstalled(c *C) {
379 // whats installed
380- parts, err := s.systemImage.GetInstalled()
381+ parts, err := s.systemImage.Installed()
382 c.Assert(err, IsNil)
383 // we have one active and one inactive
384 c.Assert(len(parts), Equals, 2)
385@@ -262,16 +262,16 @@
386 c.Assert(parts[1].Version(), Equals, "3.14")
387 }
388
389-func (s *SITestSuite) TestGetUpdateNoUpdate(c *C) {
390- parts, err := s.systemImage.GetUpdates()
391+func (s *SITestSuite) TestUpdateNoUpdate(c *C) {
392+ parts, err := s.systemImage.Updates()
393 c.Assert(err, IsNil)
394 c.Assert(len(parts), Equals, 0)
395 }
396
397-func (s *SITestSuite) TestGetUpdateHasUpdate(c *C) {
398+func (s *SITestSuite) TestUpdateHasUpdate(c *C) {
399 // add a update
400 s.mockSystemImage.info["target_build_number"] = "3.14"
401- parts, err := s.systemImage.GetUpdates()
402+ parts, err := s.systemImage.Updates()
403 c.Assert(err, IsNil)
404 c.Assert(len(parts), Equals, 1)
405 c.Assert(parts[0].Name(), Equals, "ubuntu-core")
406@@ -308,7 +308,7 @@
407 func (s *SITestSuite) TestSystemImagePartInstallUpdatesPartition(c *C) {
408 // add a update
409 s.mockSystemImage.info["target_build_number"] = "3.14"
410- parts, err := s.systemImage.GetUpdates()
411+ parts, err := s.systemImage.Updates()
412
413 sp := parts[0].(*SystemImagePart)
414 mockPartition := MockPartition{}
415@@ -322,7 +322,7 @@
416 func (s *SITestSuite) TestSystemImagePartInstall(c *C) {
417 // add a update
418 s.mockSystemImage.info["target_build_number"] = "3.14"
419- parts, err := s.systemImage.GetUpdates()
420+ parts, err := s.systemImage.Updates()
421
422 sp := parts[0].(*SystemImagePart)
423 mockPartition := MockPartition{}

Subscribers

People subscribed via source and target branches