Merge c9d26fd into 9421a5d

.build/flags.mk

@@ -3,7 +3,11 @@ PKGS ?= $(shell glide novendor)
 LIST_PKGS ?= $(shell go list ./... | grep -v /vendor/)
 
 # Many Go tools take file globs or directories as arguments instead of packages.
-PKG_FILES ?= *.go auth config dig internal metrics modules service testutils tracing ulog
+ROOT_PKG_FILES := $(wildcard *.go)
+# Get all the go packages, ignore vendor and take fx/* directories
+GLIDE_NV := $(shell glide nv)
+PKG_NAMES := $(shell go list $(GLIDE_NV) | grep -v "fx/examples" |  cut -d"/" -f 3 | uniq | paste -sd " " -)
+PKG_FILES ?= $(ROOT_PACKAGE_FILES) $(PKG_NAMES)
 
 # The linting tools evolve with each Go version, so run them only on the latest
 # stable release.

.build/lint.mk

@@ -22,11 +22,12 @@ lint:
 	@echo "Checking formatting..."
 	$(ECHO_V)gofmt -d -s $(PKG_FILES) 2>&1 | tee $(LINT_LOG)
 	@echo "Checking vet..."
-	$(ECHO_V)$(foreach dir,$(PKG_FILES),go tool vet $(VET_RULES) $(dir) 2>&1 | $(FILTER_LINT) | tee -a $(LINT_LOG);)
+	$(ECHO_V)go tool vet $(VET_RULES) $(ROOT_PKG_FILES) 2>&1 | $(FILTER_LINT) | tee -a $(LINT_LOG)
+	$(ECHO_V)go tool vet $(VET_RULES) $(PKG_NAMES) 2>&1 | $(FILTER_LINT) | tee -a $(LINT_LOG)
 	@echo "Checking lint..."
 	$(ECHO_V)$(foreach dir,$(PKGS),golint $(dir) 2>&1 | $(FILTER_LINT) | tee -a $(LINT_LOG);)
 	@echo "Checking unchecked errors..."
-	$(ECHO_V)$(foreach dir,$(PKGS),errcheck $(ERRCHECK_FLAGS) $(dir) 2>&1 | $(FILTER_LINT) | tee -a $(LINT_LOG);)
+	$(ECHO_V)errcheck $(ERRCHECK_FLAGS) $(PKGS) 2>&1 | $(FILTER_LINT) | tee -a $(LINT_LOG)
 	@echo "Checking for unresolved FIXMEs..."
 	$(ECHO_V)git grep -i fixme | grep -v -e vendor -e $(_THIS_MAKEFILE) -e CONTRIBUTING.md | tee -a $(LINT_LOG)
 	@echo "Checking for license headers..."

CHANGELOG.md

@@ -2,11 +2,16 @@
 
 ## v1.0.0-beta3 (unreleased)
 
+* Add `task.MustRegister` convenience function which fails fast by panicking
+  Note that this should only be used during app initialization, and is provided
+  to avoid repetetive error checking for services which register many tasks.
+
 ## v1.0.0-beta2 (09 Mar 2017)
 
 * [Breaking] Remove `ulog.Logger` interface and expose `*zap.Logger` directly.
 * [Breaking] Rename config and module from `modules.rpc` to `modules.yarpc`
-* [Breaking] Rename config key from `modules.http` to `modules.uhttp` to match the module name
+* [Breaking] Rename config key from `modules.http` to `modules.uhttp` to match
+  the module name
 * [Breaking] Upgrade `zap` to `v1.0.0-rc.3` (now go.uber.org/zap, was
     github.com/uber-go/zap)
 * Remove now-unused `config.IsDevelopmentEnv()` helper to encourage better
@@ -23,7 +28,8 @@
   as it is not thrift-specific.
 * Report version metrics for company-wide version usage information.
 * Allow configurable service name and module name via service options.
-* DIG constructors now support returning a tuple with the second argument being an error.
+* DIG constructors now support returning a tuple with the second argument being
+  an error.
 
 ## v1.0.0-beta1 (20 Feb 2017)
 

config/decoder.go

@@ -200,13 +200,6 @@ func (d *decoder) mapping(childKey string, value reflect.Value, def string) erro
 	}
 
 	val := v.Value()
-
-	// We fallthrough for interface to maps
-	if valueType.Kind() == reflect.Interface {
-		value.Set(reflect.ValueOf(val))
-		return nil
-	}
-
 	destMap := reflect.ValueOf(reflect.MakeMap(valueType).Interface())
 
 	// child yamlNode parsed from yaml file is of type map[interface{}]interface{}
@@ -231,6 +224,22 @@ func (d *decoder) mapping(childKey string, value reflect.Value, def string) erro
 	return nil
 }
 
+// Sets value to an interface type.
+func (d *decoder) iface(key string, value reflect.Value, def string) error {
+	v := d.getGlobalProvider().Get(key)
+	if !v.HasValue() || v.Value() == nil {
+		return nil
+	}
+
+	src := reflect.ValueOf(v.Value())
+	if src.Type().Implements(value.Type()) {
+		value.Set(src)
+		return nil
+	}
+
+	return fmt.Errorf("%q doesn't implement %q", src.Type(), value.Type())
+}
+
 // Sets value to an object type.
 func (d *decoder) object(childKey string, value reflect.Value) error {
 	value = value.Addr()
@@ -374,7 +383,7 @@ func (d *decoder) unmarshal(name string, value reflect.Value, def string) error
 	case reflect.Map:
 		return d.mapping(name, value, def)
 	case reflect.Interface:
-		return d.mapping(name, value, def)
+		return d.iface(name, value, def)
 	case reflect.Chan:
 		return d.channel(name, value, def)
 	case reflect.Func:

config/yaml_test.go

@@ -718,6 +718,10 @@ Song: off my cloud
 	for k, v := range tests {
 		t.Run(k, func(*testing.T) { v() })
 	}
+
+	for k, v := range tests {
+		t.Run(k, func(*testing.T) { v() })
+	}
 }
 
 func TestGrumpyUnmarshallerChannelFunction(t *testing.T) {

modules/task/execution.go

@@ -133,6 +133,15 @@ func Register(fn interface{}) error {
 	return nil
 }
 
+// MustRegister registers a function for an async task or panics
+// Since Task registration is performed in main() we want to fail-fast and
+// reduce the amount of error-checking
+func MustRegister(fn interface{}) {
+	if err := Register(fn); err != nil {
+		panic(err)
+	}
+}
+
 // Run decodes the message and executes as a task
 func Run(ctx context.Context, message []byte) error {
 	stopwatch := globalBackendStatsClient().TaskExecutionTime().Start()

modules/task/execution_test.go

@@ -95,11 +95,22 @@ func TestRegisterFnWithMismatchedArgCount(t *testing.T) {
 	assert.Contains(t, err.Error(), "2 function arg(s) but found 0")
 }
 
+func TestMustRegisterPanicsOnBadType(t *testing.T) {
+	assert.Panics(t, func() {
+		MustRegister("not a function")
+	})
+}
+
+func TestMustRegisterOkOnGoodFunc(t *testing.T) {
+	assert.NotPanics(t, func() {
+		MustRegister(validTaskFunc)
+	})
+}
+
 func TestEnqueueFnWithMismatchedArgType(t *testing.T) {
-	fn := func(ctx context.Context, s string) error { return nil }
-	err := Register(fn)
+	err := Register(validTaskFunc)
 	require.NoError(t, err)
-	err = Enqueue(fn, _ctx, 1)
+	err = Enqueue(validTaskFunc, _ctx, 1)
 	require.Error(t, err)
 	assert.Contains(
 		t, err.Error(), "argument: 2 from type: int to type: string",
@@ -269,3 +280,7 @@ func TestCastToError(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "be error but found")
 }
+
+func validTaskFunc(ctx context.Context, s string) error {
+	return nil
+}