From 1dfd6f131dd127d5ead1f8c9b1654ea9b314d715 Mon Sep 17 00:00:00 2001
From: at15 <at15@outlook.com>
Date: Tue, 17 Apr 2018 00:18:14 -0700
Subject: [PATCH 1/5] [doc] Add global doc directory

- add `directory.md`
- godoc will update the doc if the go file changes
  - I was thinking it just scan run it once and cache everything, it
seems it would check modification time, though the godoc server don't
have any logging enabled (by default) ...
---
 .ayi.yml         | 19 -------------------
 .gitattributes   | 19 -------------------
 Makefile         |  3 ++-
 README.md        |  5 ++---
 doc/directory.md | 15 +++++++++++++++
 doc/env.md       |  1 +
 doc/style.md     | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 log/README.md    |  9 +++++++--
 log/doc.go       | 13 +++++++++++++
 log/pkg.go       |  4 +++-
 10 files changed, 89 insertions(+), 45 deletions(-)
 delete mode 100644 .ayi.yml
 delete mode 100644 .gitattributes
 create mode 100644 doc/directory.md
 create mode 100644 doc/env.md
 create mode 100644 doc/style.md
 create mode 100644 log/doc.go

diff --git a/.ayi.yml b/.ayi.yml
deleted file mode 100644
index 6d5b036..0000000
--- a/.ayi.yml
+++ /dev/null
@@ -1,19 +0,0 @@
-dep-install:
-    - dep ensure
-test:
-    - go test -v -cover ./cast/... ./config/... ./errors/... ./generator/... ./log/... ./noodle/... ./requests/... ./structure/... ./util/...
-scripts:
-  fmt:
-    - gofmt -d -l -w ./cast ./config ./errors ./log ./requests ./structure ./util
-    - git status
-  check:
-    # TODO: didn't see any error so far ...
-    # TODO: other checks, go vet, golint etc.
-    - errcheck ./config ./log
-  doc:
-    - xdg-open http://localhost:6060/pkg/github.com/dyweb/gommon
-    # NOTE: should start godoc first and then run xdg-open, but currently Ayi can't run command in parallel or background
-    - godoc -http=":6060"
-  docker-test:
-    - docker-compose -f scripts/docker-compose.yml run --rm golang1.9
-    - docker-compose -f scripts/docker-compose.yml run --rm golanglatest
\ No newline at end of file
diff --git a/.gitattributes b/.gitattributes
deleted file mode 100644
index 3073a1f..0000000
--- a/.gitattributes
+++ /dev/null
@@ -1,19 +0,0 @@
-# disable auto crlf
-* text=false
-
-# force lf for sh, go, yml
-*.sh text eol=lf
-*.go eol=lf
-.gitignore eol=lf
-.gitattributes eol=lf
-*.yml eol=lf
-*.md eol=lf
-*.html eol=lf
-*.js eol=lf
-*.ts eol=lf
-*.css eol=lf
-*.json eol=lf
-
-# force encoding for sh, go
-*.sh encoding=utf-8
-*.go encoding=utf-8
diff --git a/Makefile b/Makefile
index 5d43fc6..72f6a87 100644
--- a/Makefile
+++ b/Makefile
@@ -21,11 +21,12 @@ test-log:
 fmt:
 	gofmt -d -l -w $(PKGST)
 
+# TODO: refer tools used in https://github.com/360EntSecGroup-Skylar/goreporter
 .PHONY: vet
 vet:
 	go vet $(PKGST)
 
-.PHONy: doc
+.PHONY: doc
 doc:
 	xdg-open http://localhost:6060/pkg/github.com/dyweb/gommon &
 	godoc -http=":6060"
diff --git a/README.md b/README.md
index a5faeca..8d6f8aa 100644
--- a/README.md
+++ b/README.md
@@ -24,7 +24,6 @@ Legacy
 - [log v1](legacy/log) A logrus like structured logger
 - [Runner](legacy/runner) A os/exec wrapper
 
-
 ## Dependencies
 
 Currently we only have one non standard library dependencies, see [Gopkg.lock](Gopkg.lock)
@@ -34,7 +33,7 @@ Currently we only have one non standard library dependencies, see [Gopkg.lock](G
     - might write one in [ANTLR](https://github.com/antlr/antlr4)
   - we also have a DSL work in progress [RCL: Reika Configuration Language](https://github.com/at15/reika/issues/49), which is like [HCL](https://github.com/hashicorp/hcl2)
 
-Removed 
+Removed
 
 - [pkg/errors](https://github.com/pkg/errors) for including context in error
   - removed in [#59](https://github.com/dyweb/gommon/pull/59)
@@ -101,7 +100,7 @@ config
 
 requests
 
-- [Requests](http://docs.python-requests.org/en/master/) for requests
+- [python requests](http://docs.python-requests.org/en/master/) for requests
 - [hashicorp/go-cleanhttp](https://github.com/hashicorp/go-cleanhttp) for using non default http transport and client
 
 generator
diff --git a/doc/directory.md b/doc/directory.md
new file mode 100644
index 0000000..33dc528
--- /dev/null
+++ b/doc/directory.md
@@ -0,0 +1,15 @@
+# Directory
+
+- [cmd/gommon](../cmd/gommon) the command line application
+- [cast](../cast) converter needed for config
+- [config](../config) config file reader
+- [doc](.) documentation
+- [errors](../errors) error wrapping and multi error
+- [generator](../generator) generating interface methods, render go template, protobuf etc.
+- [legacy](../legacy) legacy code base
+- [noodle](../noodle) embed static assets for go binary with .ignore file support
+- [playground](../playground) test library and replay issues
+- [requests](../requests) http util
+- [scripts](../scripts) test scripts
+- [structure](../structure) data structure
+- [util](../util) util packages for public use, they are too small to be top level package
\ No newline at end of file
diff --git a/doc/env.md b/doc/env.md
new file mode 100644
index 0000000..99ad436
--- /dev/null
+++ b/doc/env.md
@@ -0,0 +1 @@
+# Develop environment setup
\ No newline at end of file
diff --git a/doc/style.md b/doc/style.md
new file mode 100644
index 0000000..8103dfd
--- /dev/null
+++ b/doc/style.md
@@ -0,0 +1,46 @@
+# Coding Style
+
+The coding style can be split into two parts, application and library,
+gommon is mainly a set of libraries, but it also contains a command line application with same name `gommon`.
+
+## Folder structure
+
+see [directory](directory.md)
+
+## Documentation
+
+MUST cover the following
+
+- [ ] convention, i.e. variable names, error handling etc.
+- [ ] internal, a basic walk through of import parts
+
+## Application
+
+### Application error handling
+
+- when using `log.Fatal`, add a `return` after it, it won't get executed, but it makes the abort logic more obvious
+
+Good
+
+````go
+if cfg.Port < 0 {
+    log.Fatalf("invalid port number %d", cfg.Port)
+    return
+}
+server.Serve(cfg.Port)
+````
+
+Bad
+
+````go
+if cfg.Port < 0 {
+    log.Fatalf("invalid port number %d", cfg.Port)
+}
+server.Serve(cfg.Port)
+````
+
+## Library
+
+### Library error handling
+
+- DO NOT use `log.Fatal`, `panic`, always return error, if an error is added later, many application won't compile
\ No newline at end of file
diff --git a/log/README.md b/log/README.md
index 970575c..238d04c 100644
--- a/log/README.md
+++ b/log/README.md
@@ -1,8 +1,13 @@
 # Log
 
-## Usage 
+## Convention
 
-all the logger of a package should be registered in a registry, which is also a logger
+- library/application MUST have a library/application logger as registry.
+- every package MUST have a package level logger.
+- logger is a registry and can contain children.
+- instance of struct should have their own logger as children of package logger
+
+## Usage
 
 ````go
 package logutil
diff --git a/log/doc.go b/log/doc.go
new file mode 100644
index 0000000..8287528
--- /dev/null
+++ b/log/doc.go
@@ -0,0 +1,13 @@
+// Package log provides structured logging with fine grained control
+// over libraries using a tree hierarchy of loggers
+//
+// Conventions
+//
+// library/application MUST have a library/application logger as registry.
+//
+// every package MUST have a package level logger.
+//
+// logger is a registry and can contain children.
+//
+// instance of struct should have their own logger as children of package logger
+package log
diff --git a/log/pkg.go b/log/pkg.go
index 0382132..76e200e 100644
--- a/log/pkg.go
+++ b/log/pkg.go
@@ -1,10 +1,12 @@
-// Package log is a structured logging with fine grained control
 package log
 
 //// TODO: deal w/ http access log later
 //type HttpAccessLogger struct {
 //}
 
+// LoggableStruct is used to inject a logger into the struct, the methods for the interface can and should be generated using gommon.
+//
+// In struct initializer call dlog.NewStruct(pkgLogger, structInstancePointer)
 type LoggableStruct interface {
 	GetLogger() *Logger
 	SetLogger(logger *Logger)

From 45f59b973d7cf01bbda23fcb26c32ef25afaa4da Mon Sep 17 00:00:00 2001
From: at15 <at15@outlook.com>
Date: Wed, 2 May 2018 21:14:46 -0700
Subject: [PATCH 2/5] [doc][log] Add example for log level string

- [ ] TODO: can't write example for colored string, \x1b does not work
---
 config/yaml_test.go |  1 +
 log/TODO.md         |  4 ++++
 log/doc.go          | 28 ++++++++++++++++------------
 log/field.go        |  4 +++-
 log/handler.go      | 42 +++++++++++++++++++++++++++++++-----------
 log/level.go        | 27 ++++++++++++++++++++++++---
 log/level_test.go   | 15 +++++++++++++++
 log/pkg.go          | 11 ++++++-----
 8 files changed, 100 insertions(+), 32 deletions(-)

diff --git a/config/yaml_test.go b/config/yaml_test.go
index 2a9f4d8..a9c879c 100644
--- a/config/yaml_test.go
+++ b/config/yaml_test.go
@@ -9,6 +9,7 @@ import (
 	asst "github.com/stretchr/testify/assert"
 )
 
+// Deprecated this only works for old logging package
 type logConfig struct {
 	Level  string `yaml:"level"`
 	Color  bool   `yaml:"color"`
diff --git a/log/TODO.md b/log/TODO.md
index 045dc46..49d5dfd 100644
--- a/log/TODO.md
+++ b/log/TODO.md
@@ -1,5 +1,9 @@
 # TODO
 
+## 2018-05-02
+
+- [ ] [#67](https://github.com/dyweb/gommon/issues/67) add example in doc 
+
 ## 2018-02-04
 
 - [x] support fields
diff --git a/log/doc.go b/log/doc.go
index 8287528..cf54357 100644
--- a/log/doc.go
+++ b/log/doc.go
@@ -1,13 +1,17 @@
-// Package log provides structured logging with fine grained control
-// over libraries using a tree hierarchy of loggers
-//
-// Conventions
-//
-// library/application MUST have a library/application logger as registry.
-//
-// every package MUST have a package level logger.
-//
-// logger is a registry and can contain children.
-//
-// instance of struct should have their own logger as children of package logger
+/*
+Package log provides structured logging with fine grained control
+over libraries using a tree hierarchy of loggers
+
+Conventions
+
+1. no direct use of the log package, MUST create new logger.
+
+2. library/application MUST have a library/application logger as their registry.
+
+3. every package MUST have a package level logger as child of the registry, normally defined in pkg.go
+
+4. logger is a registry and can contain children.
+
+5. instance of struct should have their own logger as children of package logger
+*/
 package log
diff --git a/log/field.go b/log/field.go
index 42b92a5..e0388ed 100644
--- a/log/field.go
+++ b/log/field.go
@@ -1,6 +1,8 @@
 package log
 
-import "fmt"
+import (
+	"fmt"
+)
 
 type FieldType uint8
 
diff --git a/log/handler.go b/log/handler.go
index a53af15..d3a606d 100644
--- a/log/handler.go
+++ b/log/handler.go
@@ -12,6 +12,10 @@ const (
 	defaultTimeStampFormat = time.RFC3339
 )
 
+// Handler formats log message and writes to underlying storage, stdout, file, remote server etc.
+// It MUST be thread safe because logger call handler concurrently witout any locking
+// There is NO log entry struct in gommon/log, which is used in many logging packages, the reason is
+// if extra field is added to the interface, compiler would throw error on stale handler implementation.
 type Handler interface {
 	HandleLog(level Level, time time.Time, msg string)
 	HandleLogWithSource(level Level, time time.Time, msg string, source string)
@@ -29,6 +33,10 @@ type Handler interface {
 //	f(level, msg)
 //}
 
+var _ Syncer = (*os.File)(nil)
+
+// Syncer is implemented by os.File, handler implementation should check this interface and call Sync
+// if they support using file as sink
 type Syncer interface {
 	Sync() error
 }
@@ -39,10 +47,12 @@ type ioHandler struct {
 
 var defaultHandler = &ioHandler{w: os.Stderr}
 
+// DefaultHandler returns the singleton defaultHandler instance, which logs to stdout in text format
 func DefaultHandler() Handler {
 	return defaultHandler
 }
 
+// NewIOHanlder
 func NewIOHandler(w io.Writer) Handler {
 	return &ioHandler{w: w}
 }
@@ -91,7 +101,7 @@ func (h *ioHandler) Flush() {
 	}
 }
 
-// unlike log v1 entry is only used for test, it is not passed around
+// entry is only used for test, it is not passed around like other loging packages
 type entry struct {
 	level  Level
 	time   time.Time
@@ -100,46 +110,56 @@ type entry struct {
 	source string
 }
 
-var _ Handler = (*testHandler)(nil)
+var _ Handler = (*TestHandler)(nil)
 
-type testHandler struct {
+// TestHandler stores log as entry, its slice is protected by a RWMutex and safe for concurrent use
+type TestHandler struct {
 	mu      sync.RWMutex
 	entries []entry
 }
 
-func NewTestHandler() *testHandler {
-	return &testHandler{}
+// NewTestHandler returns a test handler, it should only be used in test,
+// a concrete type instead of Handler interface is returned to reduce unnecessary type cast in test
+func NewTestHandler() *TestHandler {
+	return &TestHandler{}
 }
 
-func (h *testHandler) HandleLog(level Level, time time.Time, msg string) {
+// HandleLog implements Handler interface
+func (h *TestHandler) HandleLog(level Level, time time.Time, msg string) {
 	h.mu.Lock()
 	h.entries = append(h.entries, entry{level: level, time: time, msg: msg})
 	h.mu.Unlock()
 }
 
-func (h *testHandler) HandleLogWithSource(level Level, time time.Time, msg string, source string) {
+// HandleLogWithSource implements Handler interface
+func (h *TestHandler) HandleLogWithSource(level Level, time time.Time, msg string, source string) {
 	h.mu.Lock()
 	h.entries = append(h.entries, entry{level: level, time: time, msg: msg, source: source})
 	h.mu.Unlock()
 }
 
-func (h *testHandler) HandleLogWithFields(level Level, time time.Time, msg string, fields Fields) {
+// HandleLogWithFields implements Handler interface
+func (h *TestHandler) HandleLogWithFields(level Level, time time.Time, msg string, fields Fields) {
 	h.mu.Lock()
 	h.entries = append(h.entries, entry{level: level, time: time, msg: msg, fields: fields})
 	h.mu.Unlock()
 }
 
-func (h *testHandler) HandleLogWithSourceFields(level Level, time time.Time, msg string, source string, fields Fields) {
+// HandleLogWithSourceFields implements Handler interface
+func (h *TestHandler) HandleLogWithSourceFields(level Level, time time.Time, msg string, source string, fields Fields) {
 	h.mu.Lock()
 	h.entries = append(h.entries, entry{level: level, time: time, msg: msg, source: source, fields: fields})
 	h.mu.Unlock()
 }
 
-func (h *testHandler) Flush() {
+// Flush implements Handler interface
+func (h *TestHandler) Flush() {
 	// nop
 }
 
-func (h *testHandler) HasLog(level Level, msg string) bool {
+// HasLog checks if a log with specified level and message exists in slice
+// TODO: support field, source etc.
+func (h *TestHandler) HasLog(level Level, msg string) bool {
 	h.mu.RLock()
 	defer h.mu.RUnlock()
 	for _, e := range h.entries {
diff --git a/log/level.go b/log/level.go
index 86c8a56..29155e3 100644
--- a/log/level.go
+++ b/log/level.go
@@ -3,23 +3,26 @@ package log
 import "github.com/dyweb/gommon/util/color"
 
 // Level is log level
+// TODO: allow change default logging level at compile time
 type Level uint8
 
 const (
 	// FatalLevel log error and call `os.Exit(1)`
-	// TODO: allow user hook exit?
+	// TODO: allow user to add hooks before calling os.Exit?
 	FatalLevel Level = iota
 	// PanicLevel log error and call `panic`
 	PanicLevel
-	// ErrorLevel log error
+	// ErrorLevel log error and do nothing
+	// TODO: add integration with errors package
 	ErrorLevel
-	// WarnLevel log warning
+	// WarnLevel log warning that is often ignored
 	WarnLevel
 	// InfoLevel log info
 	InfoLevel
 	// DebugLevel log debug message, user should enable DebugLevel logging when report bug
 	DebugLevel
 	// TraceLevel is very verbose, user should enable it only on packages they are currently investing instead of globally
+	// TODO: add compile flag to use empty trace logger implementation to eliminate the call at runtime
 	TraceLevel
 )
 
@@ -34,6 +37,16 @@ var levelStrings = []string{
 	TraceLevel: "trace",
 }
 
+var levelAlignedUpperStrings = []string{
+	FatalLevel: "FATA",
+	PanicLevel: "PANI",
+	ErrorLevel: "ERRO",
+	WarnLevel:  "WARN",
+	InfoLevel:  "INFO",
+	DebugLevel: "DEBU",
+	TraceLevel: "TRAC",
+}
+
 var levelColoredStrings = []string{
 	FatalLevel: color.RedStart + "fatal" + color.End,
 	PanicLevel: color.RedStart + "panic" + color.End,
@@ -58,10 +71,18 @@ func (level Level) String() string {
 	return levelStrings[level]
 }
 
+// AlignedUpperString returns fixed length level string in uppercase
+func (level Level) AlignedUpperString() string {
+	return levelAlignedUpperStrings[level]
+}
+
+// ColoredString returns level string wrapped by terminal color characters, only works on *nix
 func (level Level) ColoredString() string {
 	return levelColoredStrings[level]
 }
 
+// ColoredAlignedUpperString returns fixed length level string in uppercase,
+// wrapped by terminal color characters, only works on *nix
 func (level Level) ColoredAlignedUpperString() string {
 	return levelColoredAlignedUpperStrings[level]
 }
diff --git a/log/level_test.go b/log/level_test.go
index cd4f8a2..3b0c12b 100644
--- a/log/level_test.go
+++ b/log/level_test.go
@@ -28,3 +28,18 @@ func TestLevel_String(t *testing.T) {
 		assert.Equal(c.Str, fmt.Sprint(c.Lvl))
 	}
 }
+
+func ExampleLevel() {
+	fmt.Println(FatalLevel.String())
+	fmt.Println(FatalLevel.AlignedUpperString())
+	// Output:
+	// fatal
+	// FATA
+}
+
+// FIXME: how to write the terminal color in example output, need to debug godoc command, it does not have any output
+//func ExampleLevel_ColoredString() {
+//	fmt.Println(FatalLevel.ColoredString())
+//	// Output:
+//	// \x1b[31mfatal\x1b[0m
+//}
\ No newline at end of file
diff --git a/log/pkg.go b/log/pkg.go
index 76e200e..5b0e956 100644
--- a/log/pkg.go
+++ b/log/pkg.go
@@ -1,10 +1,7 @@
 package log
 
-//// TODO: deal w/ http access log later
-//type HttpAccessLogger struct {
-//}
-
-// LoggableStruct is used to inject a logger into the struct, the methods for the interface can and should be generated using gommon.
+// LoggableStruct is used to inject a logger into the struct,
+// the methods for the interface can and should be generated using gommon.
 //
 // In struct initializer call dlog.NewStruct(pkgLogger, structInstancePointer)
 type LoggableStruct interface {
@@ -12,3 +9,7 @@ type LoggableStruct interface {
 	SetLogger(logger *Logger)
 	LoggerIdentity(justCallMe func() *Identity) *Identity
 }
+
+//// TODO: deal w/ http access log later
+//type HttpAccessLogger struct {
+//}

From 42d4f5d176b8f0edaf289b9c969fb8190ecfb1e4 Mon Sep 17 00:00:00 2001
From: at15 <at15@outlook.com>
Date: Thu, 3 May 2018 16:33:47 -0700
Subject: [PATCH 3/5] [doc][log] Add logger prefix to file names

- factory -> logger_factory
- identity -> logger_identity
---
 log/doc.go                                    |  2 +-
 log/doc/zap.md                                |  8 ++++
 log/field.go                                  |  8 +++-
 log/handler.go                                | 43 +++++++++++++------
 log/handlers/json/handler.go                  |  4 ++
 log/level_test.go                             |  2 +-
 log/logger.go                                 | 21 +++++++--
 log/{factory.go => logger_factory.go}         |  0
 log/{identity.go => logger_identity.go}       |  1 +
 ...entity_test.go => logger_identity_test.go} |  6 +--
 log/logger_tree.go                            | 11 +++--
 log/pkg.go                                    |  4 --
 12 files changed, 79 insertions(+), 31 deletions(-)
 rename log/{factory.go => logger_factory.go} (100%)
 rename log/{identity.go => logger_identity.go} (97%)
 rename log/{identity_test.go => logger_identity_test.go} (95%)

diff --git a/log/doc.go b/log/doc.go
index cf54357..8cea4f0 100644
--- a/log/doc.go
+++ b/log/doc.go
@@ -1,6 +1,6 @@
 /*
 Package log provides structured logging with fine grained control
-over libraries using a tree hierarchy of loggers
+over libraries using a tree hierarchy of loggers.
 
 Conventions
 
diff --git a/log/doc/zap.md b/log/doc/zap.md
index 73ef6a4..19acc05 100644
--- a/log/doc/zap.md
+++ b/log/doc/zap.md
@@ -245,6 +245,14 @@ func putCheckedEntry(ce *CheckedEntry) {
 Field field.go `Any(key string, value interface{}) zapcore.Field` and `zapcore/field.go` `func (f Field) AddTo(enc ObjectEncoder)`
 
 ````go
+type Field struct {
+	Key       string
+	Type      FieldType
+	Integer   int64
+	String    string
+	Interface interface{}
+}
+
 // Any takes a key and an arbitrary value and chooses the best way to represent
 // them as a field, falling back to a reflection-based approach only if
 // necessary.
diff --git a/log/field.go b/log/field.go
index e0388ed..02032f3 100644
--- a/log/field.go
+++ b/log/field.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 )
 
+// FieldType avoids calling reflection
 type FieldType uint8
 
 const (
@@ -12,9 +13,11 @@ const (
 	StringType
 )
 
+// Fields is a slice of Field
 type Fields []Field
 
-// TODO: we can specify the type in field ... how zap do it, using pointer?
+// Field is based on uber-go/zap https://github.com/uber-go/zap/blob/master/zapcore/field.go
+// It can be treated as a Union, the value is stored in either Int, Str or Interface
 type Field struct {
 	Key       string
 	Type      FieldType
@@ -23,6 +26,7 @@ type Field struct {
 	Interface interface{}
 }
 
+// Int creates a field with int value, it uses int64 internally
 func Int(k string, v int) Field {
 	return Field{
 		Key:  k,
@@ -31,6 +35,7 @@ func Int(k string, v int) Field {
 	}
 }
 
+// Str creates a field with string value
 func Str(k string, v string) Field {
 	return Field{
 		Key:  k,
@@ -39,6 +44,7 @@ func Str(k string, v string) Field {
 	}
 }
 
+// Stringer calls the String() method and stores return value
 func Stringer(k string, v fmt.Stringer) Field {
 	return Field{
 		Key:  k,
diff --git a/log/handler.go b/log/handler.go
index d3a606d..38c8f62 100644
--- a/log/handler.go
+++ b/log/handler.go
@@ -13,15 +13,22 @@ const (
 )
 
 // Handler formats log message and writes to underlying storage, stdout, file, remote server etc.
-// It MUST be thread safe because logger call handler concurrently witout any locking
+// It MUST be thread safe because logger calls handler concurrently without any locking.
 // There is NO log entry struct in gommon/log, which is used in many logging packages, the reason is
-// if extra field is added to the interface, compiler would throw error on stale handler implementation.
+// if extra field is added to the interface, compiler would throw error on stale handler implementations.
 type Handler interface {
+	// HandleLog accepts level, log time, formatted log message
 	HandleLog(level Level, time time.Time, msg string)
+	// HandleLogWithSource accepts formatted source line of log i.e., http.go:13
+	// TODO: pass frame instead of string so handler can use trace for error handling?
 	HandleLogWithSource(level Level, time time.Time, msg string, source string)
+	// HandleLogWithFields accepts fields with type hint,
+	// implementation should inspect the type field instead of using reflection
 	// TODO: pass pointer for fields?
 	HandleLogWithFields(level Level, time time.Time, msg string, fields Fields)
+	// HandleLogWithSourceFields accepts both source and fields
 	HandleLogWithSourceFields(level Level, time time.Time, msg string, source string, fields Fields)
+	// Flush writes the buffer to underlying storage
 	Flush()
 }
 
@@ -41,20 +48,25 @@ type Syncer interface {
 	Sync() error
 }
 
-type ioHandler struct {
-	w io.Writer
-}
+//// TODO: handler for http access log, this should be in extra package
+//type HttpAccessLogger struct {
+//}`
 
-var defaultHandler = &ioHandler{w: os.Stderr}
+var defaultHandler = NewIOHandler(os.Stderr)
 
 // DefaultHandler returns the singleton defaultHandler instance, which logs to stdout in text format
 func DefaultHandler() Handler {
 	return defaultHandler
 }
 
-// NewIOHanlder
+// IOHandler writes log to io.Writer, default handler uses os.Stderr
+type IOHandler struct {
+	w io.Writer
+}
+
+// NewIOHandler
 func NewIOHandler(w io.Writer) Handler {
-	return &ioHandler{w: w}
+	return &IOHandler{w: w}
 }
 
 // TODO: performance (which is not a major concern now ...)
@@ -66,19 +78,22 @@ func NewIOHandler(w io.Writer) Handler {
 // - what would happen if os.Stderr.Close()
 // - os.Stderr.Sync() will there be any different if stderr/stdout is redirected to a file
 
-func (h *ioHandler) HandleLog(level Level, time time.Time, msg string) {
+// HandleLog implements Handler interface
+func (h *IOHandler) HandleLog(level Level, time time.Time, msg string) {
 	b := formatHead(level, time, msg)
 	b = append(b, '\n')
 	h.w.Write(b)
 }
 
-func (h *ioHandler) HandleLogWithSource(level Level, time time.Time, msg string, source string) {
+// HandleLogWithSource implements Handler interface
+func (h *IOHandler) HandleLogWithSource(level Level, time time.Time, msg string, source string) {
 	b := formatHeadWithSource(level, time, msg, source)
 	b = append(b, '\n')
 	h.w.Write(b)
 }
 
-func (h *ioHandler) HandleLogWithFields(level Level, time time.Time, msg string, fields Fields) {
+// HandleLogWithFields implements Handler interface
+func (h *IOHandler) HandleLogWithFields(level Level, time time.Time, msg string, fields Fields) {
 	// we use raw slice instead of bytes buffer because we need to use strconv.Append*, which requires raw slice
 	b := formatHead(level, time, msg)
 	b = append(b, ' ')
@@ -87,7 +102,8 @@ func (h *ioHandler) HandleLogWithFields(level Level, time time.Time, msg string,
 	h.w.Write(b)
 }
 
-func (h *ioHandler) HandleLogWithSourceFields(level Level, time time.Time, msg string, source string, fields Fields) {
+// HandleLogWithSourceFields implements Handler interface
+func (h *IOHandler) HandleLogWithSourceFields(level Level, time time.Time, msg string, source string, fields Fields) {
 	b := formatHeadWithSource(level, time, msg, source)
 	b = append(b, ' ')
 	b = formatFields(b, fields)
@@ -95,7 +111,8 @@ func (h *ioHandler) HandleLogWithSourceFields(level Level, time time.Time, msg s
 	h.w.Write(b)
 }
 
-func (h *ioHandler) Flush() {
+// Flush implements Handler interface
+func (h *IOHandler) Flush() {
 	if s, ok := h.w.(Syncer); ok {
 		s.Sync()
 	}
diff --git a/log/handlers/json/handler.go b/log/handlers/json/handler.go
index 0c5a0a6..b2587fd 100644
--- a/log/handlers/json/handler.go
+++ b/log/handlers/json/handler.go
@@ -1,3 +1,7 @@
+/*
+Package json writes log in JSON format, it concatenates string directly and does not use encoding/json.
+TODO: support escape https://medium.com/go-walkthrough/go-walkthrough-encoding-json-package-9681d1d37a8f
+*/
 package json
 
 import (
diff --git a/log/level_test.go b/log/level_test.go
index 3b0c12b..743c21a 100644
--- a/log/level_test.go
+++ b/log/level_test.go
@@ -42,4 +42,4 @@ func ExampleLevel() {
 //	fmt.Println(FatalLevel.ColoredString())
 //	// Output:
 //	// \x1b[31mfatal\x1b[0m
-//}
\ No newline at end of file
+//}
diff --git a/log/logger.go b/log/logger.go
index 4da765f..5c6b415 100644
--- a/log/logger.go
+++ b/log/logger.go
@@ -9,17 +9,30 @@ import (
 	"time"
 )
 
+// Logger is a concrete type instead of interface because most logic is in handler.
+// There is NO lock when calling logging methods, handlers may have locks.
+// Lock is used when updating logger attributes like Level.
+//
+// For Printf style logging (Levelf), Logger formats string using fmt.Sprintf before passing it to handlers.
+// 	logger.Debugf("id is %d", id)
+// For structual logging (LevelF), Logger passes fields to handlers without any processing.
+//	logger.DebugF("hi", log.Fields{log.Str("foo", "bar")})
+// If you want to mix two styles, call fmt.Sprintf before calling DebugF,
+// 	logger.DebugF(fmt.Sprintf("id is %d", id), log.Fields{log.Str("foo", "bar")})
 type Logger struct {
-	mu       sync.RWMutex
-	h        Handler
-	level    Level
-	fields   Fields // TODO: the Fields in logger are never used, we are using DebugF to pass temporary fields
+	mu    sync.RWMutex
+	h     Handler
+	level Level
+	// TODO: Fields in logger are never used, we are using DebugF to pass temporary fields
+	// which does not allow inherit fields from parent logger
+	//fields   Fields
 	children map[string][]*Logger
 	source   bool
 	id       *Identity
 }
 
 func (l *Logger) Level() Level {
+	// TODO: might use the mutex here?
 	return l.level
 }
 
diff --git a/log/factory.go b/log/logger_factory.go
similarity index 100%
rename from log/factory.go
rename to log/logger_factory.go
diff --git a/log/identity.go b/log/logger_identity.go
similarity index 97%
rename from log/identity.go
rename to log/logger_identity.go
index e58869e..9249a7d 100644
--- a/log/identity.go
+++ b/log/logger_identity.go
@@ -7,6 +7,7 @@ import (
 	"github.com/dyweb/gommon/util/runtimeutil"
 )
 
+// LoggerType can be used for filtering loggers, it is set when creating logger
 type LoggerType uint8
 
 const (
diff --git a/log/identity_test.go b/log/logger_identity_test.go
similarity index 95%
rename from log/identity_test.go
rename to log/logger_identity_test.go
index 6539b63..8a1efd1 100644
--- a/log/identity_test.go
+++ b/log/logger_identity_test.go
@@ -44,7 +44,7 @@ func TestNewPackageLogger(t *testing.T) {
 	assert.Equal(PackageLogger, id.Type)
 	assert.Equal("pkg", id.Type.String())
 	assert.Equal("init", id.Function)
-	assert.Equal(testutil.GOPATH()+"/src/github.com/dyweb/gommon/log/identity_test.go:11",
+	assert.Equal(testutil.GOPATH()+"/src/github.com/dyweb/gommon/log/logger_identity_test.go:11",
 		fmt.Sprintf("%s:%d", id.File, id.Line))
 }
 
@@ -63,7 +63,7 @@ func TestNewStructLogger(t *testing.T) {
 	assert.Equal("struct", id.Type.String())
 	assert.Equal("Foo", id.Struct)
 	assert.Equal(MagicStructLoggerFunctionName, id.Function)
-	assert.Equal(testutil.GOPATH()+"/src/github.com/dyweb/gommon/log/identity_test.go:31",
+	assert.Equal(testutil.GOPATH()+"/src/github.com/dyweb/gommon/log/logger_identity_test.go:31",
 		fmt.Sprintf("%s:%d", id.File, id.Line))
 }
 
@@ -77,6 +77,6 @@ func TestNewMethodLogger(t *testing.T) {
 	assert.Equal("method", id.Type.String())
 	assert.Equal("Foo", id.Struct)
 	assert.Equal("method", id.Function)
-	assert.Equal(testutil.GOPATH()+"/src/github.com/dyweb/gommon/log/identity_test.go:35",
+	assert.Equal(testutil.GOPATH()+"/src/github.com/dyweb/gommon/log/logger_identity_test.go:35",
 		fmt.Sprintf("%s:%d", id.File, id.Line))
 }
diff --git a/log/logger_tree.go b/log/logger_tree.go
index 1e53baf..f7d984c 100644
--- a/log/logger_tree.go
+++ b/log/logger_tree.go
@@ -7,6 +7,7 @@ import (
 	"github.com/dyweb/gommon/structure"
 )
 
+//
 // TODO: allow release a child logger, this will be a trouble if we created 1,000 Client struct with its own logger...
 func (l *Logger) AddChild(child *Logger) {
 	l.mu.Lock()
@@ -48,6 +49,7 @@ func SetHandlerRecursive(root *Logger, handler Handler) {
 	})
 }
 
+// FIXME: change the typo, this also requires update in go.ice
 func EnableSourceRecusrive(root *Logger) {
 	visited := make(map[*Logger]bool)
 	PreOrderDFS(root, visited, func(l *Logger) {
@@ -114,11 +116,12 @@ func (l *Logger) PrintTree() {
 
 // FIXME: print tree is still having problem ....
 //⇒  icehubd log
-//app logger /home/at15/workspace/src/github.com/at15/go.ice/_example/github/pkg/util/logutil/pkg.go:8
-//└── lib logger /home/at15/workspace/src/github.com/at15/go.ice/ice/util/logutil/pkg.go:8
-//		└── lib logger /home/at15/workspace/src/github.com/dyweb/gommon/util/logutil/pkg.go:7
-//│         └── pkg logger /home/at15/workspace/src/github.com/dyweb/gommon/config/pkg.go:8
+//	app logger /home/at15/workspace/src/github.com/at15/go.ice/_example/github/pkg/util/logutil/pkg.go:8
+//	└── lib logger /home/at15/workspace/src/github.com/at15/go.ice/ice/util/logutil/pkg.go:8
+//			└── lib logger /home/at15/workspace/src/github.com/dyweb/gommon/util/logutil/pkg.go:7
+//	│         └── pkg logger /home/at15/workspace/src/github.com/dyweb/gommon/config/pkg.go:8
 
+// PrintTreeTo prints logger as a tree, using current logger as root
 func (l *Logger) PrintTreeTo(w io.Writer) {
 	st := ToStringTree(l)
 	st.PrintTo(w)
diff --git a/log/pkg.go b/log/pkg.go
index 5b0e956..848a836 100644
--- a/log/pkg.go
+++ b/log/pkg.go
@@ -9,7 +9,3 @@ type LoggableStruct interface {
 	SetLogger(logger *Logger)
 	LoggerIdentity(justCallMe func() *Identity) *Identity
 }
-
-//// TODO: deal w/ http access log later
-//type HttpAccessLogger struct {
-//}

From 6b21c4ba60be954926d24220276e1c00123b464e Mon Sep 17 00:00:00 2001
From: at15 <at15@outlook.com>
Date: Thu, 3 May 2018 17:34:08 -0700
Subject: [PATCH 4/5] [doc] Add canonical import paths

- add example for errors package
---
 cast/pkg.go                      |  2 +-
 config/pkg.go                    |  2 +-
 doc/style.md                     |  1 +
 errors/errors_test.go            |  8 ++++++++
 errors/multi_test.go             | 10 ++++++++++
 errors/pkg.go                    |  2 +-
 generator/pkg.go                 |  2 +-
 log/handlers/cli/handler.go      |  7 ++++++-
 log/handlers/cli/handler_test.go |  5 ++---
 log/handlers/json/handler.go     |  4 ++--
 log/logger.go                    | 14 +++++++++++---
 log/logger_identity.go           |  5 +++--
 log/logger_identity_test.go      |  9 +++++++++
 log/logger_tree.go               |  4 ++--
 log/pkg.go                       |  2 +-
 noodle/pkg.go                    |  2 +-
 requests/pkg.go                  |  2 +-
 structure/doc_test.go            |  9 ---------
 structure/{doc.go => pkg.go}     |  2 +-
 structure/priority_queue.go      |  2 +-
 structure/set_test.go            | 17 ++++++++++++-----
 util/color/pkg.go                |  2 +-
 util/fsutil/pkg.go               |  2 +-
 util/hashutil/pkg.go             |  2 +-
 util/logutil/pkg.go              |  6 ++++--
 util/pkg.go                      |  2 +-
 util/runtimeutil/pkg.go          |  2 +-
 util/testutil/pkg.go             |  2 +-
 28 files changed, 85 insertions(+), 44 deletions(-)
 delete mode 100644 structure/doc_test.go
 rename structure/{doc.go => pkg.go} (53%)

diff --git a/cast/pkg.go b/cast/pkg.go
index 953b788..5da9965 100644
--- a/cast/pkg.go
+++ b/cast/pkg.go
@@ -1,4 +1,4 @@
 // Package cast convert types safely and drop incompatible types during conversion
 //
 // it is inspired by https://github.com/spf13/cast
-package cast
+package cast // import "github.com/dyweb/gommon/cast"
diff --git a/config/pkg.go b/config/pkg.go
index 8c131db..c437621 100644
--- a/config/pkg.go
+++ b/config/pkg.go
@@ -1,5 +1,5 @@
 // Package config supports go text/template, environment and self defined variables
-package config
+package config // import "github.com/dyweb/gommon/config"
 
 import (
 	"github.com/dyweb/gommon/util/logutil"
diff --git a/doc/style.md b/doc/style.md
index 8103dfd..7e6406d 100644
--- a/doc/style.md
+++ b/doc/style.md
@@ -13,6 +13,7 @@ MUST cover the following
 
 - [ ] convention, i.e. variable names, error handling etc.
 - [ ] internal, a basic walk through of import parts
+- define canonical import path in `pkg.go` https://golang.org/doc/go1.4#canonicalimports 
 
 ## Application
 
diff --git a/errors/errors_test.go b/errors/errors_test.go
index a3ac621..015aa89 100644
--- a/errors/errors_test.go
+++ b/errors/errors_test.go
@@ -59,3 +59,11 @@ func TestWrap(t *testing.T) {
 	}
 	assert.Equal(4, len(terr.ErrorStack().Frames()))
 }
+
+// TODO: we should write test in errors_test package, especially for examples ...
+func ExampleWrap() {
+	err := Wrap(os.ErrNotExist, "oops")
+	fmt.Println(err)
+	// Output:
+	// oops: file does not exist
+}
diff --git a/errors/multi_test.go b/errors/multi_test.go
index 52746ba..c059f03 100644
--- a/errors/multi_test.go
+++ b/errors/multi_test.go
@@ -1,6 +1,7 @@
 package errors
 
 import (
+	"fmt"
 	"os"
 	"sync"
 	"testing"
@@ -76,3 +77,12 @@ func TestMultiErr_ErrorOrNil(t *testing.T) {
 	merr.Append(os.ErrPermission)
 	assert.NotNil(merr.ErrorOrNil())
 }
+
+func ExampleMultiErr() {
+	err := NewMultiErr()
+	err.Append(os.ErrPermission)
+	err.Append(os.ErrNotExist)
+	fmt.Println(err.Errors())
+	// Output:
+	// [permission denied file does not exist]
+}
diff --git a/errors/pkg.go b/errors/pkg.go
index 902fa5a..0daf7bf 100644
--- a/errors/pkg.go
+++ b/errors/pkg.go
@@ -1,5 +1,5 @@
 // Package errors provides multi error, error wrapping. It defines error category code for machine post processing
-package errors
+package errors // import "github.com/dyweb/gommon/errors"
 
 import "fmt"
 
diff --git a/generator/pkg.go b/generator/pkg.go
index a4cb18e..3466e1c 100644
--- a/generator/pkg.go
+++ b/generator/pkg.go
@@ -1,5 +1,5 @@
 // Package generator render go template, call external commands, generate gommon specific methods based on gommon.yml
-package generator
+package generator // import "github.com/dyweb/gommon/generator"
 
 import (
 	"github.com/dyweb/gommon/util/logutil"
diff --git a/log/handlers/cli/handler.go b/log/handlers/cli/handler.go
index 46f172b..59ca84f 100644
--- a/log/handlers/cli/handler.go
+++ b/log/handlers/cli/handler.go
@@ -1,4 +1,9 @@
-package cli
+/*
+Package cli writes is same as builtin IOHandler except color and delta time.
+It is used by go.ice as default handler
+TODO: color can't be disabled and we don't detect tty like logrus
+*/
+package cli // import "github.com/dyweb/gommon/log/handlers/cli"
 
 import (
 	"io"
diff --git a/log/handlers/cli/handler_test.go b/log/handlers/cli/handler_test.go
index 060f535..a6f26a9 100644
--- a/log/handlers/cli/handler_test.go
+++ b/log/handlers/cli/handler_test.go
@@ -1,15 +1,14 @@
 package cli
 
 import (
-	"fmt"
 	"testing"
 
 	asst "github.com/stretchr/testify/assert"
 )
 
-// TODO: test ....
 func TestHandler_HandleLog(t *testing.T) {
-	fmt.Printf("%04d", 2)
+	// FIXME: the test seems to be broken, it has no output and when run in GoLand it shows terminated instead of pass
+	//fmt.Printf("%04d", 2)
 }
 
 func Test_FormatNum(t *testing.T) {
diff --git a/log/handlers/json/handler.go b/log/handlers/json/handler.go
index b2587fd..3930a56 100644
--- a/log/handlers/json/handler.go
+++ b/log/handlers/json/handler.go
@@ -1,8 +1,8 @@
 /*
 Package json writes log in JSON format, it concatenates string directly and does not use encoding/json.
-TODO: support escape https://medium.com/go-walkthrough/go-walkthrough-encoding-json-package-9681d1d37a8f
+TODO: support escape
 */
-package json
+package json // import "github.com/dyweb/gommon/log/handlers/json"
 
 import (
 	"github.com/dyweb/gommon/log"
diff --git a/log/logger.go b/log/logger.go
index 5c6b415..0d0cf05 100644
--- a/log/logger.go
+++ b/log/logger.go
@@ -60,10 +60,13 @@ func (l *Logger) DisableSource() {
 	l.mu.Unlock()
 }
 
+// Identity returns the identity set when the logger is created.
+// NOTE: caller can modify the identity because all fields are public, but they should NOT do this
 func (l *Logger) Identity() *Identity {
 	return l.id
 }
 
+// Panic calls panic after it writes and flushes the log
 func (l *Logger) Panic(args ...interface{}) {
 	s := fmt.Sprint(args...)
 	if !l.source {
@@ -75,6 +78,7 @@ func (l *Logger) Panic(args ...interface{}) {
 	panic(s)
 }
 
+// Panicf duplicates instead of calling Panic to keep source line correct
 func (l *Logger) Panicf(format string, args ...interface{}) {
 	s := fmt.Sprintf(format, args...)
 	if !l.source {
@@ -86,6 +90,7 @@ func (l *Logger) Panicf(format string, args ...interface{}) {
 	panic(s)
 }
 
+// PanicF duplicates instead of calling Panic to keep source line correct
 func (l *Logger) PanicF(msg string, fields Fields) {
 	if !l.source {
 		l.h.HandleLogWithFields(PanicLevel, time.Now(), msg, fields)
@@ -96,6 +101,7 @@ func (l *Logger) PanicF(msg string, fields Fields) {
 	panic(msg)
 }
 
+// Fatal calls os.Exit(1) after it writes and flushes the log
 func (l *Logger) Fatal(args ...interface{}) {
 	s := fmt.Sprint(args...)
 	if !l.source {
@@ -108,7 +114,7 @@ func (l *Logger) Fatal(args ...interface{}) {
 	os.Exit(1)
 }
 
-// FIXME: source line is in correct because we call Fatal in Fatalf
+// Fatalf duplicates instead of calling Fatal to keep source line correct
 func (l *Logger) Fatalf(format string, args ...interface{}) {
 	s := fmt.Sprintf(format, args...)
 	if !l.source {
@@ -117,10 +123,10 @@ func (l *Logger) Fatalf(format string, args ...interface{}) {
 		l.h.HandleLogWithSource(FatalLevel, time.Now(), s, caller())
 	}
 	l.h.Flush()
-	// TODO: allow user to register hook to do cleanup before exit directly
 	os.Exit(1)
 }
 
+// FatalF duplicates instead of calling Fatal to keep source line correct
 func (l *Logger) FatalF(msg string, fields Fields) {
 	if !l.source {
 		l.h.HandleLogWithFields(FatalLevel, time.Now(), msg, fields)
@@ -128,10 +134,12 @@ func (l *Logger) FatalF(msg string, fields Fields) {
 		l.h.HandleLogWithSourceFields(FatalLevel, time.Now(), msg, caller(), fields)
 	}
 	l.h.Flush()
-	// TODO: allow user to register hook to do cleanup before exit directly
 	os.Exit(1)
 }
 
+// caller gets source location at runtime, in the future we may generate it at compile time to reduce the
+// overhead, though I am not sure what the overhead is without actual benchmark and profiling
+// TODO: https://github.com/dyweb/gommon/issues/43
 func caller() string {
 	_, file, line, ok := runtime.Caller(2)
 	if !ok {
diff --git a/log/logger_identity.go b/log/logger_identity.go
index 9249a7d..d5907c0 100644
--- a/log/logger_identity.go
+++ b/log/logger_identity.go
@@ -34,9 +34,9 @@ func (tpe LoggerType) String() string {
 	return loggerTypeStrings[tpe]
 }
 
-// Identity is based where the logger is initialized, it is NOT exactly where the log happens.
+// Identity is set based on logger's initialization location,
+// it is close to, but NOT exactly same as location of actual log.
 // It is used for applying filter rules and print logger hierarchy.
-// TODO: example
 type Identity struct {
 	Package  string
 	Function string
@@ -61,6 +61,7 @@ func NewIdentityFromCaller(skip int) *Identity {
 		st       string
 	)
 	tpe := UnknownLogger
+	// TODO: does it handle vendor correctly
 	pkg, function = runtimeutil.SplitPackageFunc(frame.Function)
 	tpe = FunctionLogger
 	// NOTE: we distinguish a struct logger and method logger using the magic name,
diff --git a/log/logger_identity_test.go b/log/logger_identity_test.go
index 8a1efd1..920f96e 100644
--- a/log/logger_identity_test.go
+++ b/log/logger_identity_test.go
@@ -80,3 +80,12 @@ func TestNewMethodLogger(t *testing.T) {
 	assert.Equal(testutil.GOPATH()+"/src/github.com/dyweb/gommon/log/logger_identity_test.go:35",
 		fmt.Sprintf("%s:%d", id.File, id.Line))
 }
+
+func ExampleNewApplicationLogger() {
+	logger := NewApplicationLogger()
+	fmt.Println(logger.Identity().Package)
+	fmt.Println(logger.Identity().Type)
+	// Output:
+	// github.com/dyweb/gommon/log
+	// app
+}
diff --git a/log/logger_tree.go b/log/logger_tree.go
index f7d984c..0fe5f28 100644
--- a/log/logger_tree.go
+++ b/log/logger_tree.go
@@ -49,8 +49,8 @@ func SetHandlerRecursive(root *Logger, handler Handler) {
 	})
 }
 
-// FIXME: change the typo, this also requires update in go.ice
-func EnableSourceRecusrive(root *Logger) {
+// FIXME: this fixed typo requires update in go.ice
+func EnableSourceRecursive(root *Logger) {
 	visited := make(map[*Logger]bool)
 	PreOrderDFS(root, visited, func(l *Logger) {
 		l.EnableSource()
diff --git a/log/pkg.go b/log/pkg.go
index 848a836..3491c0d 100644
--- a/log/pkg.go
+++ b/log/pkg.go
@@ -1,4 +1,4 @@
-package log
+package log // import "github.com/dyweb/gommon/log"
 
 // LoggableStruct is used to inject a logger into the struct,
 // the methods for the interface can and should be generated using gommon.
diff --git a/noodle/pkg.go b/noodle/pkg.go
index e63eca0..8a6dab8 100644
--- a/noodle/pkg.go
+++ b/noodle/pkg.go
@@ -1,5 +1,5 @@
 // Package noodle helps embedding static assets into go binary, it supports using ignore file
-package noodle
+package noodle // import "github.com/dyweb/gommon/noodle"
 
 import (
 	"github.com/dyweb/gommon/util/logutil"
diff --git a/requests/pkg.go b/requests/pkg.go
index b08b15d..9bb1d36 100644
--- a/requests/pkg.go
+++ b/requests/pkg.go
@@ -1,2 +1,2 @@
 // Package requests is a wrapper around net/http with less public global variables
-package requests
+package requests // import "github.com/dyweb/gommon/requests"
diff --git a/structure/doc_test.go b/structure/doc_test.go
deleted file mode 100644
index 2f3c30b..0000000
--- a/structure/doc_test.go
+++ /dev/null
@@ -1,9 +0,0 @@
-package structure
-
-import "fmt"
-
-func ExampleNewSet() {
-	s := NewSet("a", "b", "c")
-	fmt.Println(s.Size())
-	// Output: 3
-}
diff --git a/structure/doc.go b/structure/pkg.go
similarity index 53%
rename from structure/doc.go
rename to structure/pkg.go
index 61698c9..0e9d7ce 100644
--- a/structure/doc.go
+++ b/structure/pkg.go
@@ -1,4 +1,4 @@
 /*
 Package structure add some missing common data structures to Golang
 */
-package structure
+package structure // import "github.com/dyweb/gommon/structure"
diff --git a/structure/priority_queue.go b/structure/priority_queue.go
index 57281cc..8fa44fd 100644
--- a/structure/priority_queue.go
+++ b/structure/priority_queue.go
@@ -1,3 +1,3 @@
 package structure
 
-// TODO
+// TODO: interface & impl
diff --git a/structure/set_test.go b/structure/set_test.go
index 634af42..d4dd48d 100644
--- a/structure/set_test.go
+++ b/structure/set_test.go
@@ -1,32 +1,33 @@
 package structure
 
 import (
+	"fmt"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	asst "github.com/stretchr/testify/assert"
 )
 
 func TestSet_Contains(t *testing.T) {
-	assert := assert.New(t)
+	assert := asst.New(t)
 	s := NewSet("a", "b", "c")
 	assert.True(s.Contains("a"))
 	assert.False(s.Contains("d"))
 }
 
 func TestSet_Cardinality(t *testing.T) {
-	assert := assert.New(t)
+	assert := asst.New(t)
 	s := NewSet("a", "b", "c")
 	assert.Equal(3, s.Cardinality())
 }
 
 func TestSet_Size(t *testing.T) {
-	assert := assert.New(t)
+	assert := asst.New(t)
 	s := NewSet("a", "b", "c")
 	assert.Equal(s.Size(), s.Cardinality())
 }
 
 func TestSet_Equal(t *testing.T) {
-	assert := assert.New(t)
+	assert := asst.New(t)
 	s := NewSet("a", "b", "c")
 	s2 := NewSet("a")
 	s3 := NewSet("a")
@@ -34,3 +35,9 @@ func TestSet_Equal(t *testing.T) {
 	assert.False(s.Equal(s2))
 	assert.True(s2.Equal(s3))
 }
+
+func ExampleNewSet() {
+	s := NewSet("a", "b", "c")
+	fmt.Println(s.Size())
+	// Output: 3
+}
diff --git a/util/color/pkg.go b/util/color/pkg.go
index 9b3d354..a097617 100644
--- a/util/color/pkg.go
+++ b/util/color/pkg.go
@@ -1,2 +1,2 @@
 // Package color defines color code for pretty print in linux shell, windows is not supported
-package color
+package color // import "github.com/dyweb/gommon/util/color"
diff --git a/util/fsutil/pkg.go b/util/fsutil/pkg.go
index 73ec398..b75ba2f 100644
--- a/util/fsutil/pkg.go
+++ b/util/fsutil/pkg.go
@@ -1,5 +1,5 @@
 // Package fsutil adds ignore support for walk
-package fsutil
+package fsutil // import "github.com/dyweb/gommon/util/fsutil"
 
 import (
 	"github.com/dyweb/gommon/util/logutil"
diff --git a/util/hashutil/pkg.go b/util/hashutil/pkg.go
index b5fff72..cf1f7af 100644
--- a/util/hashutil/pkg.go
+++ b/util/hashutil/pkg.go
@@ -1,5 +1,5 @@
 // Package hashutil provides alloc free alternatives for pkg/hash
-package hashutil
+package hashutil // import "github.com/dyweb/gommon/util/hashutil"
 
 func HashStringFnv64a(str string) uint64 {
 	h := NewInlineFNV64a()
diff --git a/util/logutil/pkg.go b/util/logutil/pkg.go
index 4181ec2..89971b3 100644
--- a/util/logutil/pkg.go
+++ b/util/logutil/pkg.go
@@ -1,5 +1,7 @@
-// Package logutil is a registry of loggers, it is required for all lib and app that use gommon/log
-package logutil
+// Package logutil is a registry of loggers, it is required for all lib and app that use gommon/log.
+// You should add the registry as child of your library/application's child if you want to control gommon libraries
+// logging behavior
+package logutil // import "github.com/dyweb/gommon/util/logutil"
 
 import (
 	"github.com/dyweb/gommon/log"
diff --git a/util/pkg.go b/util/pkg.go
index efecf6b..3e05543 100644
--- a/util/pkg.go
+++ b/util/pkg.go
@@ -1,5 +1,5 @@
 // Package util contains helpers
-package util
+package util // import "github.com/dyweb/gommon/util"
 
 func GeneratedHeader(generator string, template string) string {
 	return "// Code generated by " + generator + " from " + template + " DO NOT EDIT.\n"
diff --git a/util/runtimeutil/pkg.go b/util/runtimeutil/pkg.go
index ca6b070..e9b8c84 100644
--- a/util/runtimeutil/pkg.go
+++ b/util/runtimeutil/pkg.go
@@ -1,2 +1,2 @@
 // Package runtimeutil provides wrapper to get caller, stack etc.
-package runtimeutil
+package runtimeutil // import "github.com/dyweb/gommon/util/runtimeutil"
diff --git a/util/testutil/pkg.go b/util/testutil/pkg.go
index d15937a..1923455 100644
--- a/util/testutil/pkg.go
+++ b/util/testutil/pkg.go
@@ -1,2 +1,2 @@
 // Package testutil defines helper functions that fails test instead of return error
-package testutil
+package testutil // import "github.com/dyweb/gommon/util/testutil"

From 9f623f5569346a4de4cf1cc4412c9950f27a3948 Mon Sep 17 00:00:00 2001
From: at15 <at15@outlook.com>
Date: Thu, 3 May 2018 17:45:59 -0700
Subject: [PATCH 5/5] [cmd] Fix typo in gommon/log

---
 cmd/gommon/main.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/gommon/main.go b/cmd/gommon/main.go
index 144d3a1..9ff43e5 100644
--- a/cmd/gommon/main.go
+++ b/cmd/gommon/main.go
@@ -50,7 +50,7 @@ func parseFlags(args []string) {
 	}
 	if *verbose {
 		dlog.SetLevelRecursive(log, dlog.DebugLevel)
-		dlog.EnableSourceRecusrive(log)
+		dlog.EnableSourceRecursive(log)
 	}
 }