factor out runner from juju-core. #1

Merged
merged 1 commit into from Oct 3, 2016

Conversation

Projects
None yet
3 participants
Owner

rogpeppe commented Oct 3, 2016

We make a few changes when moving from juju-core.
In particular, there's no particular use for NewRunner to
return an interface rather than a concrete type, so we
return a concrete type like a conventional Go package.

We also make the parameters to NewRunner a struct so that we won't need
to break compatibility if we want to change them.

We change the tests so we no longer have a dependency on
github.com/juju/juju and simplify them a little.

Looks good.

mhilton approved these changes Oct 3, 2016

👍 with a couple of thoughts.

+ // RestartDelay holds the length of time the runner will
+ // wait after a worker has exited with a non-fatal error
+ // before it is restarted.
+ // If this is zero, RestartDelay will be used.
@mhilton

mhilton Oct 3, 2016

Member

Perhaps call one of these variables something different? DefaultRestartDelay perhaps?

runner.go
+ p.RestartDelay = RestartDelay
+ }
+
+ runner := &Runner{
@mhilton

mhilton Oct 3, 2016

Member

Why not make RunnerParams part of this structure so you only have to assign one thing.

factor out runner from juju-core.
We make a few changes when moving from juju-core.
In particular, there's no particular use for NewRunner to
return an interface rather than a concrete type, so we
return a concrete type like a conventional Go package.

We also make the parameters to NewRunner a struct so that we won't need
to break compatibility if we want to change them.

We change the tests so we no longer have a dependency on
github.com/juju/juju and simplify them a little.

@rogpeppe rogpeppe merged commit 8b18096 into juju:v1 Oct 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment