snap: show last refresh time #2394

Merged
merged 5 commits into from Dec 8, 2016

Conversation

Projects
None yet
3 participants
Collaborator

mvo5 commented Dec 1, 2016

Build on top of #2384

zyga approved these changes Dec 1, 2016

Looks good to me, a few minor questions. Feel free to merge if irrelevant or fixed.

client/packages.go
@@ -54,7 +54,7 @@ type Snap struct {
Prices map[string]float64 `json:"prices"`
Screenshots []Screenshot `json:"screenshots"`
- Channels map[string]*snap.Ref `json:"channels"`
+ Channels map[string]*snap.ChannelSnapInfo `json:"channels"`
@zyga

zyga Dec 1, 2016

Contributor

❤️

cmd/snap/cmd_info.go
+ for i, a := range both.Apps {
+ apps[i] = a.Name
+ }
+ fmt.Fprintf(w, "apps:\t[%s]\n", strutil.Quoted(apps))
@zyga

zyga Dec 1, 2016

Contributor

Since apps cannot have any nasty characters I was wondering why would we be quoting that. I don't mind them quoted, just curious.

strutil/strutil.go
@@ -47,6 +48,18 @@ func MakeRandomString(length int) string {
return out
}
+// Convert the given size in btes to a readable string
+func SizeToStr(size int64) string {
+ suffixes := []string{"B", "kB", "MB", "GB", "TB", "PB", "EB"}
@zyga

zyga Dec 1, 2016

Contributor

Why kB but MB? 2**10 vs 10 ** 3 aside, shouldn't those be uniformly capitalised?

LGTM, we should probably just respect the "refresh" terminology here.

Contributor

niemeyer commented Dec 1, 2016

@zyga Note all of these points are actually about the underlying #2384.

Contributor

niemeyer commented Dec 3, 2016

@mvo5 conflict here.

@mvo5 mvo5 changed the title from snap: show last update time to snap: show last refresh time Dec 5, 2016

mvo5 added some commits Dec 5, 2016

@mvo5 mvo5 merged commit b4456d8 into snapcore:master Dec 8, 2016

5 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment