verbose output #46

xiekeyang · 2016-10-21T10:00:35Z

close #14 and #22 .

This provides a contexted logger instance, which is exported from logrus, to print the information to stderr and stdout. It level the information through debug option.

Signed-off-by: xiekeyang xiekeyang@huawei.com

wking

The bones look reasonable to me, although there are a few patterns of nits (I've only commented once for each of the patterns). Longer term, I think we want to write TAP or some such for the validation output, but this PR will be useful regardless of whether validation output stays in logrus or not.

wking · 2016-10-22T06:50:00Z

cmd/oci-create-runtime-bundle/main.go

+
+	logLevel, err := logrus.ParseLevel(v.logLevelString)
+	if err != nil {
+		logrus.Fatalf("%s", err)


logrus.Fatalf("%s", err) → logrus.Fatal(err).

I prefer to only Fatalf, because it can format the output message. Some needn't be formatted, but it has no harm. And it is unnecessary to use 2 methods as Fatalf and Fatal. Unitary method is clearer for next implementation.
And I hold same opinion for other levels(Infof to Info, Debugf to Debug...).

The logrus and fmt maintainers feel like it's useful to include both formatted and unformatted printers. I think it's less surprising to use the unformatted interface when you want the default format, but it's not a big deal either way.

wking · 2016-10-22T06:52:46Z

cmd/oci-create-runtime-bundle/main.go

+	cmd.Flags().StringVar(
+		&v.logLevelString, "log-level", "info",
+		`Log level (panic, fatal, error, warn, info, or debug)`,
+	)


--log-level should be documented in the associated man page (e.g. oci-create-runtime-bundle.1.md).

Added as 2d1f26d

wking · 2016-10-22T06:55:53Z

cmd/oci-create-runtime-bundle/main.go

-		v.stdout.Printf("commit: %s", gitCommit)
-		v.stdout.Printf("spec: %s", specs.Version)
+		logrus.Infof("commit: %s", gitCommit)
+		logrus.Infof("spec: %s", specs.Version)


These are supposed to go to stdout, so we should leave them alone.

OK, but I think it is easy and clear to use fmt.Printf() directly here, WDYT?

fmt.Printf works for me. It means that there's no way to capture the stream generated by the command (which you can currently do by setting v.stdout to something other than os.Stdout), but this is in cmd/ and not in a library, so I think that decreased flexibility is fine.

wking · 2016-10-22T06:57:47Z

cmd/oci-create-runtime-bundle/main.go

 		}
-		os.Exit(1)
+		logrus.Fatalf("both src and dest must be provided")


logrus.Fatal("both src and dest must be provided") (Fatalf → Fatal).

wking · 2016-10-23T18:02:10Z

cmd/oci-create-runtime-bundle/oci-create-runtime-bundle.1.md

@@ -26,6 +26,9 @@ runtime-spec-compatible `dest/config.json`.
 **--type**
  Type of the file to unpack. If unset, oci-create-runtime-bundle will try to auto-detect the type. One of "imageLayout,image"

+**--log-level**
+  Log level to create runtime bundle. To be one of panic, fatal, error, warn, info, or debug. If unset, the level is identical to info.


I think we should follow runtime-tools and make the default level error. That covers the stuff that leads to non-zero exit codes. Folks interested in additional detail can bump up --log-level, and more restricted default allows us to be more generous about the amount of warn- and info-level logging we put in (without making the default command too noisy).

@wking
I've researched runtime tool repo, but found it has only debug messages, without info message(so far) need feed back to consumers.
But for image tools repo, there are messages, like 5beb020#diff-da07f373046a0a24e67b18699dc15a70R88 and 5beb020#diff-8c24ca9b63b3598ef3f4cc64741f433eR117, need be shown to consumers.

Later we maybe add more information, which consumer does care about, so I think default info level is better.
I'm not persistent to it, but could you please consider it? Thanks.

@wking , WDYT on above comment?

I haven't had time to look over the logging in both repos; maybe tomorrow?

All right, thanks so much.

runtime-tools has a warning in-flight with opencontainers/runtime-tools#256. I still think the default should be error. I agree that the info messages you linked to are useful output, but I'd rather handle them by printing TAP to stdout instead of by using logrus to conditionally print them to stderr. On the other hand, TAP is a big shift from where we are now; I'm fine handling that change in a follow-up PR.

@wking
Fixed it in d527098 and 0c2f6f2, PTAL.

Currently I suppose image tool to use fmt.Printf() directly as stdout (for it should be unconditional), logrus.Errorf() as stderr, and other conditional level message via logrus, which is adopted in docker/docker. But, I doubt a little if it is prefect behavior. Could you please present your understanding about TAP? Thanks a lot!

Filed the TAP thoughts in #60.

wking · 2016-10-28T03:54:08Z

glide.lock

@@ -25,6 +25,8 @@ imports:
  version: e02fc20de94c78484cd5ffb007f8af96be030a45
 - name: github.com/xeipuuv/gojsonschema
  version: d5336c75940ef31c9ceeb0ae64cf92944bccb4ee
+- name: github.com/Sirupsen/logrus
+  version: 3ec0642a7fb6488f65b06f9040adc67e3990296a


logrus cuts tagged releases. Can we use 0.10.0 here?

@wking
I fix it in 5deb77e, to use 0.10.0.
BTW, if we really need glide to fetch the vendor code? I notice that runtime-tools repo(and many popular repos) put package code into Godeps folder. I know that the benifit of glide is to reserve the necessary code accurately. But if we really want to take it, to have to install glide on each host? Maybe we should only import third party code into vendor or Godeps directly(follow up PR). WDYT?

wking · 2016-10-31T03:59:49Z

cmd/oci-create-runtime-bundle/main.go

 	if v.version {
-		v.stdout.Printf("commit: %s", gitCommit)
-		v.stdout.Printf("spec: %s", specs.Version)
+		fmt.Printf("commit: %s\n", gitCommit)


Printf → Println (then you can drop the \n addition).

Ah, nevermind. I'm fine with Printf, since Println doesn't support a format string.

wking · 2016-10-31T04:06:01Z

I'd still rather use Fatal instead of Fatalf when the format string is
"%s" 1, but other than that 5deb77e looks good to me.

xiekeyang · 2016-10-31T06:20:43Z

@wking
Fix in 7ff9b24

logrus.Fatalf("%s",err) -> logrus.Fatal(err)
logrus.Fatalf("constant string") -> logrus.Fatal("constant string")
For error level messages, I keep only using logrus.Errorf(), as I explained in verbose output #46 (comment).

wking · 2016-10-31T17:28:35Z

On Sun, Oct 30, 2016 at 11:20:43PM -0700, xiekeyang wrote:

logrus.Fatalf("%s",err) -> logrus.Fatal(err)

logrus.Fatalf("constant string") -> logrus.Fatal("constant string")

For error level messages, I keep only using logrus.Errorf(), as
I explained in
verbose output #46 (comment).

Errorf vs. Error seems the same as Fatalf vs. Fatal to me. But I'm
fine landing this as it stands and filing a follow-up PR to address
Errorf → Error in isolation.

xiekeyang · 2016-11-18T08:35:04Z

@opencontainers/image-tools-maintainers
I think this PR may be ready to merged. PTAL.

stevvooe · 2016-11-18T20:05:51Z

cmd/oci-image-validate/main.go

 		if err := cmd.Usage(); err != nil {
-			v.stderr.Println(err)
+			logrus.Errorf("%s", err)


logrus.WithError(err).Errorf("usage failed")

fixed in 9ae60ce.
To use logrus.WithError(err).Errorf() to instead logrus.Errorf(), on related positions.

stevvooe · 2016-11-18T20:06:32Z

@xiekeyang Make sure that logging is going to stderr to differentiate from problem output, which should go to stdout.

xiekeyang · 2016-11-21T03:59:01Z

@stevvooe

@xiekeyang Make sure that logging is going to stderr to differentiate from problem output, which should go to stdout.

I assume that problem output means messages consumer want to collect, like why validation failed, right? For example below use stderr,

if err := cmd.Usage(); err != nil {
        logrus.WithError(err).Errorf("usage failed")
}

And below use stdout.

err := v.validatePath(arg) 
if err != nil { 
  fmt.Printf("error: validation failed: %s",err)
}

Is that what you want to say? If so, I'm not very clear about their difference.

stevvooe · 2016-11-21T22:26:13Z

@xiekeyang Yes, that is effectively what I am saying. However, the output to stdout should have some machine readable structure.

xiekeyang · 2016-11-22T02:29:26Z

@stevvooe
I mean problem output prompted by validatePath might be open file error, json unmarshal error or so.
So, I'm not sure how to differ stderr and stdout and I worry it might loss messages when consumer collect them. I honestly fell they are all stderr. I think they are printed as stderr(to use logrus.WithError(err).Errorf()), and consumer can collect all of them by console or by audit.
I have no strong opinion, but could you reconsider it?

xiekeyang · 2016-11-30T02:48:41Z

@stevvooe
After read my comment #46 (comment), do you still want to use stdout to print "problem output", or think to use stderr to be fine?
Thanks a lot!

stevvooe · 2016-11-30T23:28:05Z

@xiekeyang stdout should have only have errors that are part of the program output. For example, if the job of the program is to validate something, the errors in validation would go to standard out. If an error occurs trying to perform the validation, that output would go to stderr.

Simply put, I would print all of logrus to stderr and then control program output through fmt.Printfln or equivalent wrapper.

xiekeyang · 2016-12-06T04:22:46Z

@stevvooe
PTAL 565f138.
Here I fix the stdout and stderr to your proposal, to use logrus for stderr, and go log(Printf and Println) for stdout.
I implement the new package logger, to provide public logger structure for each command.
If it is what you want? Thanks a lot!

stevvooe · 2016-12-06T20:28:06Z

Here I fix the stdout and stderr to your proposal, to use logrus for stderr, and go log(Printf and Println) for stdout.

This is not really what I said or meant. :/

There isn't really a point to using both the standard logger and and logrus. They are redundant.

What I said was that you need to differentiate program output, ie. validation, and status of the program with stdout and stderr respectively.

Let's say we have output, tagged with the stream (this is an example, don't add it), it would look like this:

stderr: opened manifest.json for validation
stdout: ok manifest.json
stderr: opened foo.json for validation
stdout: fail foo.json
stdout:   error bad media type
stdout:   error missing digest
stderr: failed to open notexist.json

Typically, we use a logger to write to stderr, which can be logrus to provide leveled logging.

This distinction is the hallmark of a well structured unix tool. It allows us to pipe program output somewhere, while monitoring stderr for behavior.

I hope this clarifies the requirements here. (if anyone else has a link explaining this concept, it would be helpful).

xiekeyang · 2017-01-19T02:39:47Z

@stevvooe ,

update it in b619fba.

It should have coverred what you want. PTAL, thanks a lot!

xiekeyang · 2017-01-19T12:29:07Z

And update the PR description at #46 (comment)

xiekeyang · 2017-02-03T01:50:21Z

@stevvooe @vbatts @philips PTAL.

vbatts · 2017-02-08T19:47:50Z

LGTM, but rebase is needed

xiekeyang · 2017-02-09T02:12:17Z

rebased, PTAL.

vbatts · 2017-02-09T20:30:43Z

LGTM

stevvooe · 2017-02-09T21:24:48Z

logger/logger.go

@@ -0,0 +1,62 @@
+package logger


What is the purpose of this package?

Here is an example of a context-integrated logger that is threadsafe: https://github.com/docker/containerd/blob/master/log/context.go#L28.

However, please let me know what the intent of this package and we can get it into shape.

xiekeyang · 2017-02-20T11:36:29Z

What is the purpose of this package?
However, please let me know what the intent of this package and we can get it into shape.
I'm still confused about the role of a full blown logger in a CLI tool. Shouldn't we do something more user friendly?

I'm in progress for this PR. Excuse me that I might only partly understand you. docker/containerd and docker/distribution are using individual, and public package to organize log, and provide an instance.
Yes I should wrapped it via go context, for avoiding racing purpose, and we can add more contexts in future. So do you only mean to introduce go context, or you have some another proposal hint? Thanks so much.

xiekeyang · 2017-02-20T11:36:55Z

@stevvooe

What is the purpose of this package?
However, please let me know what the intent of this package and we can get it into shape.
I'm still confused about the role of a full blown logger in a CLI tool. Shouldn't we do something more user friendly?

I'm in progress for this PR. Excuse me that I might only partly understand you. docker/containerd and docker/distribution are using individual, and public package to organize log, and provide an instance.
Yes I should wrapped it via go context, for go racing purpose, and we can add more contexts in future. So do you only mean to introduce go context, or you have some another proposal hint? Thanks so much.

stevvooe · 2017-02-22T19:44:49Z

@xiekeyang I don't understand why you need a leveled logger in a command line tool. It makes no sense.

xiekeyang · 2017-02-26T10:07:50Z

@stevvooe You might just want xx-tool -D/--debug. However we note log-level is the way on runtime-tools and docker/containerd repo. I think consumer might have the same case on image-tools. So please reconsider it. Thank lots!

stevvooe · 2017-02-28T03:10:34Z

docker/containerd repo.

Don't use this as an example: this is very new code and half of it is a daemon.

xiekeyang · 2017-02-28T03:13:08Z

@stevvooe OK. So I use it as image-tool-xx -D/--debug for logrus.Debugf print. Is that alright?

xiekeyang · 2017-03-13T09:49:27Z

@stevvooe @opencontainers/image-tools-maintainers
This is updated.

Logger instance should be retrieved through golang.org/x/net/context.
Set up debug mode for logrus.Debugf(), instead of log-level.
Fire messages to stderr and stdout devices.
PTAL, thank lots!

vbatts · 2017-03-13T12:06:34Z

cmd/oci-create-runtime-bundle/main.go

-
-	cmd := newBundleCmd(stdout, stderr)
+	ctx := context.Background()
+	ctx = context.WithLogger(ctx, context.G(ctx))


this reads in a stuttered way. Seems like it could just be a oneliner

@vbatts You are right. It is updated to 548a4ae and eb76ffb

The changeset is:
eb76ffb#diff-bd4694bbf0ec2644693b1ef195e3d330R49
eb76ffb#diff-8c24ca9b63b3598ef3f4cc64741f433eR54
eb76ffb#diff-cfb3ced180edd25cfc0a028c1a6e900dR48

Changed logentry to LogEntry as public variable. And then in */main.go it is applied as,

ctx := context.WithLogger(context.Background(), LogEntry)

That improves declaration of context to oneline, and keep context resource usage unchanged.

vbatts · 2017-03-13T12:51:24Z

i like the idea of using context for sharing the logger

cyphar · 2017-03-13T14:44:43Z

I also really like it, because it means that the CAS and other functions in umoci that will move here should in principle just be able to use the context logging (I didn't even know you could do that -- which is why I'm stuck with apex in umoci 😉).

jonboulle · 2017-03-13T16:46:26Z

context/context.go

@@ -0,0 +1,31 @@
+// Copyright 2016 The Linux Foundation


Is this package totally new or did you find it from somewhere else?

Yes, it is totally new. I intend that context package will include more functionalities besides logger, so I define a widely used name as context.

I refer this framework to docker/containerd and docker/distribution.

Don't follow the distribution pattern. We'll probably rip that out in favor the containerd/swarmkit approach.

xiekeyang · 2017-03-15T02:29:37Z

@opencontainers/image-tools-maintainers It is updated, PTAL, thanks.

vbatts · 2017-03-15T14:19:18Z

Seems like a good starting point. Thanks @xiekeyang
LGTM

stevvooe · 2017-03-15T18:53:12Z

@vbatts I still don't understand why we are using a logger for user consumed output. This still seems like the wrong direction.

stevvooe · 2017-03-15T18:55:07Z

context/logger.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package context


I'd recommend against calling this package context. It causes future problems when you try to work with both the context package and other packages. This package provides logging.

@stevvooe If logger or logging is feasible name? I worry that in future this might include more functionalities besides logging to use context value to, so that should we use generic name?

Is that referred to docker/distribution? 😄

stevvooe · 2017-03-15T18:57:35Z

glide.lock

+- name: golang.org/x/net
+  version: d379faa25cbdc04d653984913a2ceb43b0bc46d7
+  subpackages:
+  - context


I'd recommend we use the stdlib package for new interfaces.

@stevvooe Do you mean import context directly?

Please don't do that. Currently on openSUSE we haven't switched to Go 1.8 because of issues building on aarch64 (amongst other things). So using import "context" breaks for non-x86_64 architectures.

@cyphar context was added in Go 1.7. If we export interfaces that use golang.org/x/net/context, we will be stuck using that package forever.

stevvooe · 2017-03-15T18:58:39Z

context/logger.go

+type (
+	loggerKey struct{}
+
+	stderrHook struct{}


What is the role of the stderr hook? Can't we just set the logging output to go to stderr?

@stevvooe
In this design, I set default logger.Out to stdout, because I want to print info and debug to stdout.

LogEntry.Logger.Out = os.Stdout

I create hook because I can fire the output to stderr, by hook.Fire() function. And I trigger it when level = [fatal,panic,error,warn], as hook.Level() is doing. Hook is necessary for logger.

About if we should use logger, I mostly agree with @vbatts. If we use logrus directly without logger, maybe the applicability will be a little narrow (although I can not figure out all usage now).

Logging and debug should always go to stderr.

Wow, Excuse me I was not aware of it. If it is a popular way for client tools? Or could you please give me some more explain?

@stevvooe

Logging and debug should always go to stderr.

Wow, Excuse me I was not aware of it. If it is a popular way for client tools? Or could you please give me some more explain?

On Thu, Mar 16, 2017 at 04:41:49PM -0700, xiekeyang wrote:

If it is a popular way for client tools?

POSIX suggests:

… standard output (for writing conventional output), and standard error (for writing diagnostic output).

The Linux man pages have:

Under normal circumstances every UNIX program has three streams opened for it when it starts up, one for input, one for output, and one for printing diagnostic or error messages… the output stream is referred to as "standard output"; and the error stream is referred to as "standard error".

So I think “normal output to stdout and logging/debug to stderr” is a fairly well-established pattern. Deciding what is “normal output” and what is “logging/debug” may require more clarity around our intended output, but I'd guess most things which need tunable verbosity are debugging logs which should be going to stderr.

After trading off @stevvooe and @wking comment, I rather write output totally to stderr. Customer establish log system mainly because of diagnostic purpose. Definitely they want to collect output to one stream. So I will use only stderr, and clarify it in manual.

vbatts · 2017-03-15T19:00:09Z

printing out a report or a consumable json, is not the as a logging. These don't have to be blocking each other.

…

On Wed, Mar 15, 2017 at 2:53 PM, Stephen Day ***@***.***> wrote: @vbatts <https://github.com/vbatts> I still don't understand why we are using a logger for user consumed output. This still seems like the wrong direction. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#46 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEF6WFZNbRNLYrJe48-_WcphCTGTa8wks5rmDOYgaJpZM4KdECk> .

xiekeyang · 2017-03-16T11:24:11Z

I'm rebasing it. For the code uses github.com/urfave/cli, I'm research if cli.Context can transport Context.

xiekeyang · 2017-03-19T11:18:47Z

@opencontainers/image-tools-maintainers Rebased, PTAL.
Use stdlib context, and add the comment for SUSE platform in logger/logger.go. Print output to stderr.

xiekeyang · 2017-04-06T11:05:29Z

@stevvooe This should be mostly looking well and reasonable to your suggestion.

contexted logging.
only out to stderr.

PTAL, cc @opencontainers/image-tools-maintainers

This package provides contexted logging. Create logrus logger, and add it to context value. It help to print output to stderr. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

Verbose output utilizes public logger pakcage. It create command context with which logger can be retrieved. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

xiekeyang · 2017-04-07T13:44:35Z

rebased, PTAL. ping @stevvooe

stevvooe · 2017-04-28T23:11:41Z

image/image.go

 	"github.com/pkg/errors"
 )

 // ValidateLayout walks through the given file tree and validates the manifest
 // pointed to by the given refs or returns an error if the validation failed.
-func ValidateLayout(src string, refs []string, out *log.Logger) error {
-	return validate(newPathWalker(src), refs, out)
+func ValidateLayout(ctx logger.Context, src string, refs []string) error {


This doesn't make sense. This should just use a regular context. Don't define a new Context type.

Please follow the pattern in containerd. It is well-healed and does exactly what is needed here.

stevvooe · 2017-04-28T23:12:04Z

logger/context.go

+)
+
+// Context is a copy of Context from the stdlib context package.
+type Context interface {


Do not define a new context type.

wking reviewed Oct 22, 2016

View reviewed changes

wking reviewed Oct 23, 2016

View reviewed changes

wking reviewed Oct 28, 2016

View reviewed changes

wking reviewed Oct 31, 2016

View reviewed changes

stevvooe reviewed Nov 18, 2016

View reviewed changes

xiekeyang changed the title ~~use logrus to ease printing to stdout~~ [WIP] use logrus to ease printing to stdout Dec 1, 2016

xiekeyang changed the title ~~[WIP] use logrus to ease printing to stdout~~ use logrus to ease printing to stdout Dec 6, 2016

xiekeyang changed the title ~~use logrus to ease printing to stdout~~ use logrus to ease printing to stderr Dec 6, 2016

xiekeyang changed the title ~~use logrus to ease printing to stderr~~ [WIP]:use logrus to ease printing to stderr Jan 17, 2017

xiekeyang changed the title ~~[WIP]:use logrus to ease printing to stderr~~ [WIP]: provide logger via logrus Jan 17, 2017

xiekeyang changed the title ~~[WIP]: provide logger via logrus~~ provide logger to print message to stderr and stdout Jan 19, 2017

stevvooe reviewed Feb 9, 2017

View reviewed changes

xiekeyang changed the title ~~provide logger to print message to stderr and stdout~~ [WIP] provide logger to print message to stderr and stdout Feb 16, 2017

xiekeyang changed the title ~~[WIP] provide logger to print message to stderr and stdout~~ verbose output Mar 13, 2017

xiekeyang mentioned this pull request Mar 13, 2017

Merging of subcommands #103

Merged

vbatts reviewed Mar 13, 2017

View reviewed changes

jonboulle reviewed Mar 13, 2017

View reviewed changes

stevvooe reviewed Mar 15, 2017

View reviewed changes

xiekeyang mentioned this pull request Apr 6, 2017

cmd/oci-image-tool/validate.go: create a logger if none exists. #134

Merged

xiekeyang added 2 commits April 7, 2017 10:10

logger: provide contexted logging

857c8d8

This package provides contexted logging. Create logrus logger, and add it to context value. It help to print output to stderr. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

verbose output

8b10600

Verbose output utilizes public logger pakcage. It create command context with which logger can be retrieved. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

stevvooe reviewed Apr 28, 2017

View reviewed changes

verbose output #46

Are you sure you want to change the base?

verbose output #46

Conversation

xiekeyang commented Oct 21, 2016 • edited Loading

wking left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang Oct 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang Oct 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang Oct 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang Oct 30, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Oct 31, 2016

xiekeyang commented Oct 31, 2016

wking commented Oct 31, 2016

xiekeyang commented Nov 18, 2016

Choose a reason for hiding this comment

xiekeyang Nov 21, 2016 • edited Loading

Choose a reason for hiding this comment

stevvooe commented Nov 18, 2016

xiekeyang commented Nov 21, 2016

stevvooe commented Nov 21, 2016

xiekeyang commented Nov 22, 2016

xiekeyang commented Nov 30, 2016

stevvooe commented Nov 30, 2016

xiekeyang commented Dec 6, 2016 • edited Loading

stevvooe commented Dec 6, 2016

xiekeyang commented Jan 19, 2017

xiekeyang commented Jan 19, 2017

xiekeyang commented Feb 3, 2017

vbatts commented Feb 8, 2017 • edited by caniszczyk Loading

xiekeyang commented Feb 9, 2017

vbatts commented Feb 9, 2017 • edited by caniszczyk Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang commented Feb 20, 2017

xiekeyang commented Feb 20, 2017

stevvooe commented Feb 22, 2017

xiekeyang commented Feb 26, 2017

stevvooe commented Feb 28, 2017

xiekeyang commented Feb 28, 2017

xiekeyang commented Mar 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbatts commented Mar 13, 2017

cyphar commented Mar 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang commented Mar 15, 2017

vbatts commented Mar 15, 2017 • edited by caniszczyk Loading

stevvooe commented Mar 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang commented Oct 21, 2016 •

edited

Loading

xiekeyang Oct 23, 2016 •

edited

Loading

xiekeyang Oct 23, 2016 •

edited

Loading

xiekeyang Oct 28, 2016 •

edited

Loading

xiekeyang Oct 30, 2016 •

edited

Loading

xiekeyang Nov 21, 2016 •

edited

Loading

xiekeyang commented Dec 6, 2016 •

edited

Loading

vbatts commented Feb 8, 2017 •

edited by caniszczyk

Loading

vbatts commented Feb 9, 2017 •

edited by caniszczyk

Loading

vbatts commented Mar 15, 2017 •

edited by caniszczyk

Loading