Merge lp:~axwalk/juju-core/lp1211629-git-config-useremail into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

worker/uniter/charm: git config user.email on init

If we don't manually set user.email, and git can't
derive an email address from the FQDN (e.g. we don't
have one), then charms cannot be deployed.

Fixes bug #1211629

https://codereview.appspot.com/12924043/

worker/uniter/charm: git config user.email on init

If we don't manually set user.email, and git can't
derive an email address from the FQDN (e.g. we don't
have one), then charms cannot be deployed.

Fixes bug #1211629

https://codereview.appspot.com/12924043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+180066_code.launchpad.net,

Message:
Please take a look.

Description:
worker/uniter/charm: git config user.email on init

If we don't manually set user.email, and git can't
derive an email address from the FQDN (e.g. we don't
have one), then charms cannot be deployed.

Fixes bug #1211629

https://code.launchpad.net/~axwalk/juju-core/lp1211629-git-config-useremail/+merge/180066

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/12924043/

Affected files:
   A [revision details]
   M worker/uniter/charm/git.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20130809021225-6c5p05y8zs5j83o3
+New revision: <email address hidden>

Index: worker/uniter/charm/git.go
=== modified file 'worker/uniter/charm/git.go'
--- worker/uniter/charm/git.go 2013-05-02 15:55:42 +0000
+++ worker/uniter/charm/git.go 2013-08-14 08:46:26 +0000
@@ -54,7 +54,17 @@
   if err := os.MkdirAll(d.path, 0755); err != nil {
    return err
   }
- return d.cmd("init")
+ commands := [][]string{
+ {"init"},
+ {"config", "user.email", "juju@localhost."},
+ {"config", "user.name", "juju"},
+ }
+ for _, args := range commands {
+ if err := d.cmd(args...); err != nil {
+ return err
+ }
+ }
+ return nil
  }

  // AddAll ensures that the next commit will reflect the current contents of

[Nate Finch (natefinch)]

LGTM

https://codereview.appspot.com/12924043/

[William Reade (fwereade)]

not lgtm without a test, but appears sound otherwise

https://codereview.appspot.com/12924043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/12924043/

[Nate Finch (natefinch)]

not lgtm given that I'm pretty sure it's not going to compile.

https://codereview.appspot.com/12924043/diff/6001/worker/uniter/charm/git_test.go
File worker/uniter/charm/git_test.go (right):

https://codereview.appspot.com/12924043/diff/6001/worker/uniter/charm/git_test.go#newcode17
worker/uniter/charm/git_test.go:17: "regexp"
Does this compile? fmt and regexp aren't used as far as I can tell.

Also, can you sort the imports while you're up there?

https://codereview.appspot.com/12924043/

[Andrew Wilkins (axwalk)]

On 2013/08/15 13:00:54, nate.finch wrote:
> not lgtm given that I'm pretty sure it's not going to compile.

https://codereview.appspot.com/12924043/diff/6001/worker/uniter/charm/git_test.go
> File worker/uniter/charm/git_test.go (right):

https://codereview.appspot.com/12924043/diff/6001/worker/uniter/charm/git_test.go#newcode17
> worker/uniter/charm/git_test.go:17: "regexp"
> Does this compile? fmt and regexp aren't used as far as I can tell.

Sigh, I need to sleep more. Thanks for the catch... I made a tweak after
running the tests.

> Also, can you sort the imports while you're up there?

Will do.

https://codereview.appspot.com/12924043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/12924043/

[Andrew Wilkins (axwalk)]

On 2013/08/15 13:15:02, axw wrote:
> Please take a look.

Before you tell me, I'll move gc to its own line ;)

https://codereview.appspot.com/12924043/

[Nate Finch (natefinch)]

On 2013/08/15 13:16:09, axw1 wrote:
> On 2013/08/15 13:15:02, axw wrote:
> > Please take a look.

> Before you tell me, I'll move gc to its own line ;)

Awesome, and glad to see more . imports going away. LGTM.

https://codereview.appspot.com/12924043/

[Tim Penhey (thumper)]

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git.go
File worker/uniter/charm/git.go (right):

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git.go#newcode59
worker/uniter/charm/git.go:59: {"config", "user.email",
"juju@localhost."},
Why does the email address end in a dot?

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go
File worker/uniter/charm/git_test.go (left):

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#oldcode30
worker/uniter/charm/git_test.go:30: s.oldLcAll = os.Getenv("LC_ALL")
I added a testing.PatchEnvironment function that returns a simple
function.

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go
File worker/uniter/charm/git_test.go (right):

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#newcode12
worker/uniter/charm/git_test.go:12: gc "launchpad.net/gocheck"
We've gone for three sections of imports:

std libs

other imports

juju-core

So please add a blank line after gocheck.

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#newcode52
worker/uniter/charm/git_test.go:52: c.Assert(string(out), gc.Matches,
"(.|\n)*user.email=juju@localhost.\nuser.name=juju(.|\n)*")
In testing/checkers there is a Contains matcher.

c.Assert(string(out), jc.Contains, "user.email=juju@localhost.\n")
c.Assert(string(out), jc.Contains, "user.name=juju\n")

Would probably cast out prior though.

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#newcode60
worker/uniter/charm/git_test.go:60: c.Assert(exists, gc.Equals, false)
There is also IsTrue, and IsFalse in testing/checkers

https://codereview.appspot.com/12924043/

[Andrew Wilkins (axwalk)]

On 2013/08/16 01:10:06, thumper wrote:

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git.go
> File worker/uniter/charm/git.go (right):

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git.go#newcode59
> worker/uniter/charm/git.go:59: {"config", "user.email",
"juju@localhost."},
> Why does the email address end in a dot?

Because I misread the special-use domain RFC. Fixed, though apparently
git doesn't care.

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go
> File worker/uniter/charm/git_test.go (left):

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#oldcode30
> worker/uniter/charm/git_test.go:30: s.oldLcAll = os.Getenv("LC_ALL")
> I added a testing.PatchEnvironment function that returns a simple
function.

Done.

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go
> File worker/uniter/charm/git_test.go (right):

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#newcode12
> worker/uniter/charm/git_test.go:12: gc "launchpad.net/gocheck"
> We've gone for three sections of imports:

> std libs

> other imports

> juju-core

> So please add a blank line after gocheck.

Yep sorry, I just missed that in the last one - done. I'm going to write
a tool to check the import groups, and stick it in a bzr pre-commit
hook.

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#newcode52
> worker/uniter/charm/git_test.go:52: c.Assert(string(out), gc.Matches,
> "(.|\n)*user.email=juju@localhost.\nuser.name=juju(.|\n)*")
> In testing/checkers there is a Contains matcher.

> c.Assert(string(out), jc.Contains, "user.email=juju@localhost.\n")
> c.Assert(string(out), jc.Contains, "user.name=juju\n")

> Would probably cast out prior though.

Done, thanks.

https://codereview.appspot.com/12924043/diff/13001/worker/uniter/charm/git_test.go#newcode60
> worker/uniter/charm/git_test.go:60: c.Assert(exists, gc.Equals, false)
> There is also IsTrue, and IsFalse in testing/checkers

Done.

https://codereview.appspot.com/12924043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/12924043/

[Tim Penhey (thumper)]

LGTM with bonus points for updating the import :)

https://codereview.appspot.com/12924043/diff/22001/worker/uniter/charm/git_test.go
File worker/uniter/charm/git_test.go (left):

https://codereview.appspot.com/12924043/diff/22001/worker/uniter/charm/git_test.go#oldcode11
worker/uniter/charm/git_test.go:11:
"launchpad.net/juju-core/testing/checkers"
We have kind of agreed now to use:

jc "launchpad.net/juju-core/testing/checkers"

This ended up being a three way discussion between Roger, David and me,
and this is the compromise.

https://codereview.appspot.com/12924043/

[Tim Penhey (thumper)]

On 2013/08/15 09:49:54, fwereade wrote:
> not lgtm without a test, but appears sound otherwise

Formally overriding this "not merge" as it was just about missing tests,
and we have one now.

https://codereview.appspot.com/12924043/

=== modified file 'worker/uniter/charm/git.go'
--- worker/uniter/charm/git.go	2013-05-02 15:55:42 +0000
+++ worker/uniter/charm/git.go	2013-08-16 03:01:05 +0000
@@ -8,13 +8,14 @@
 	"fmt"
 	"io"
 	"io/ioutil"
-	"launchpad.net/juju-core/charm"
-	"launchpad.net/juju-core/log"
-	"launchpad.net/juju-core/utils"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+
+	"launchpad.net/juju-core/charm"
+	"launchpad.net/juju-core/log"
+	"launchpad.net/juju-core/utils"
 )
 
 var ErrConflict = errors.New("charm upgrade has conflicts")
@@ -54,7 +55,17 @@
 	if err := os.MkdirAll(d.path, 0755); err != nil {
 		return err
 	}
-	return d.cmd("init")
+	commands := [][]string{
+		{"init"},
+		{"config", "user.email", "juju@localhost"},
+		{"config", "user.name", "juju"},
+	}
+	for _, args := range commands {
+		if err := d.cmd(args...); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // AddAll ensures that the next commit will reflect the current contents of

=== modified file 'worker/uniter/charm/git_test.go'
--- worker/uniter/charm/git_test.go	2013-06-25 09:50:52 +0000
+++ worker/uniter/charm/git_test.go	2013-08-16 03:01:05 +0000
@@ -5,13 +5,16 @@
 
 import (
 	"io/ioutil"
-	. "launchpad.net/gocheck"
+	"os"
+	"os/exec"
+	"path/filepath"
+
+	gc "launchpad.net/gocheck"
+
 	corecharm "launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/testing"
-	"launchpad.net/juju-core/testing/checkers"
+	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/worker/uniter/charm"
-	"os"
-	"path/filepath"
 )
 
 var curl = corecharm.MustParseURL("cs:series/blah-blah-123")
@@ -19,206 +22,220 @@
 type GitDirSuite struct {
 	testing.GitSuite
 	LoggingSuite testing.LoggingSuite
-	oldLcAll     string
+	resetLcAll   func()
 }
 
-var _ = Suite(&GitDirSuite{})
+var _ = gc.Suite(&GitDirSuite{})
 
-func (s *GitDirSuite) SetUpTest(c *C) {
+func (s *GitDirSuite) SetUpTest(c *gc.C) {
 	s.GitSuite.SetUpTest(c)
 	s.LoggingSuite.SetUpTest(c)
-	s.oldLcAll = os.Getenv("LC_ALL")
-	os.Setenv("LC_ALL", "en_US")
+	s.resetLcAll = testing.PatchEnvironment("LC_ALL", "en_US")
 }
 
-func (s *GitDirSuite) TearDownTest(c *C) {
-	os.Setenv("LC_ALL", s.oldLcAll)
+func (s *GitDirSuite) TearDownTest(c *gc.C) {
+	s.resetLcAll()
 	s.LoggingSuite.TearDownTest(c)
 	s.GitSuite.TearDownTest(c)
 }
 
-func (s *GitDirSuite) TestCreate(c *C) {
+func (s *GitDirSuite) TestInitConfig(c *gc.C) {
+	base := c.MkDir()
+	repo := charm.NewGitDir(filepath.Join(base, "repo"))
+	err := repo.Init()
+	c.Assert(err, gc.IsNil)
+
+	cmd := exec.Command("git", "config", "--list", "--local")
+	cmd.Dir = repo.Path()
+	out, err := cmd.Output()
+	c.Assert(err, gc.IsNil)
+	outstr := string(out)
+	c.Assert(outstr, jc.Contains, "user.email=juju@localhost")
+	c.Assert(outstr, jc.Contains, "user.name=juju")
+}
+
+func (s *GitDirSuite) TestCreate(c *gc.C) {
 	base := c.MkDir()
 	repo := charm.NewGitDir(filepath.Join(base, "repo"))
 	exists, err := repo.Exists()
-	c.Assert(err, IsNil)
-	c.Assert(exists, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(exists, jc.IsFalse)
 
 	err = ioutil.WriteFile(repo.Path(), nil, 0644)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	_, err = repo.Exists()
-	c.Assert(err, ErrorMatches, `".*/repo" is not a directory`)
+	c.Assert(err, gc.ErrorMatches, `".*/repo" is not a directory`)
 	err = os.Remove(repo.Path())
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 
 	err = os.Chmod(base, 0555)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	defer os.Chmod(base, 0755)
 	err = repo.Init()
-	c.Assert(err, ErrorMatches, ".* permission denied")
+	c.Assert(err, gc.ErrorMatches, ".* permission denied")
 	exists, err = repo.Exists()
-	c.Assert(err, IsNil)
-	c.Assert(exists, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(exists, jc.IsFalse)
 
 	err = os.Chmod(base, 0755)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = repo.Init()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	exists, err = repo.Exists()
-	c.Assert(err, IsNil)
-	c.Assert(exists, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(exists, jc.IsTrue)
 
 	_, err = charm.ReadCharmURL(repo)
-	c.Assert(err, checkers.Satisfies, os.IsNotExist)
+	c.Assert(err, jc.Satisfies, os.IsNotExist)
 
 	err = repo.Init()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 }
 
-func (s *GitDirSuite) TestAddCommitPullRevert(c *C) {
+func (s *GitDirSuite) TestAddCommitPullRevert(c *gc.C) {
 	target := charm.NewGitDir(c.MkDir())
 	err := target.Init()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = ioutil.WriteFile(filepath.Join(target.Path(), "initial"), []byte("initial"), 0644)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = charm.WriteCharmURL(target, curl)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = target.AddAll()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	dirty, err := target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsTrue)
 	err = target.Commitf("initial")
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	dirty, err = target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsFalse)
 
 	source := newRepo(c)
 	err = target.Pull(source)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	url, err := charm.ReadCharmURL(target)
-	c.Assert(err, IsNil)
-	c.Assert(url, DeepEquals, curl)
+	c.Assert(err, gc.IsNil)
+	c.Assert(url, gc.DeepEquals, curl)
 	fi, err := os.Stat(filepath.Join(target.Path(), "some-dir"))
-	c.Assert(err, IsNil)
-	c.Assert(fi, checkers.Satisfies, os.FileInfo.IsDir)
+	c.Assert(err, gc.IsNil)
+	c.Assert(fi, jc.Satisfies, os.FileInfo.IsDir)
 	data, err := ioutil.ReadFile(filepath.Join(target.Path(), "some-file"))
-	c.Assert(err, IsNil)
-	c.Assert(string(data), Equals, "hello")
+	c.Assert(err, gc.IsNil)
+	c.Assert(string(data), gc.Equals, "hello")
 	dirty, err = target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsFalse)
 
 	err = ioutil.WriteFile(filepath.Join(target.Path(), "another-file"), []byte("blah"), 0644)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	dirty, err = target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsTrue)
 	err = source.AddAll()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	dirty, err = target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsTrue)
 
 	err = target.Revert()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	_, err = os.Stat(filepath.Join(target.Path(), "some-file"))
-	c.Assert(err, checkers.Satisfies, os.IsNotExist)
+	c.Assert(err, jc.Satisfies, os.IsNotExist)
 	_, err = os.Stat(filepath.Join(target.Path(), "some-dir"))
-	c.Assert(err, checkers.Satisfies, os.IsNotExist)
+	c.Assert(err, jc.Satisfies, os.IsNotExist)
 	data, err = ioutil.ReadFile(filepath.Join(target.Path(), "initial"))
-	c.Assert(err, IsNil)
-	c.Assert(string(data), Equals, "initial")
+	c.Assert(err, gc.IsNil)
+	c.Assert(string(data), gc.Equals, "initial")
 	dirty, err = target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsFalse)
 }
 
-func (s *GitDirSuite) TestClone(c *C) {
+func (s *GitDirSuite) TestClone(c *gc.C) {
 	repo, err := newRepo(c).Clone(c.MkDir())
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	_, err = os.Stat(filepath.Join(repo.Path(), "some-file"))
-	c.Assert(err, checkers.Satisfies, os.IsNotExist)
+	c.Assert(err, jc.Satisfies, os.IsNotExist)
 	_, err = os.Stat(filepath.Join(repo.Path(), "some-dir"))
-	c.Assert(err, checkers.Satisfies, os.IsNotExist)
+	c.Assert(err, jc.Satisfies, os.IsNotExist)
 	dirty, err := repo.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsTrue)
 
 	err = repo.AddAll()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	dirty, err = repo.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsTrue)
 	err = repo.Commitf("blank overwrite")
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	dirty, err = repo.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsFalse)
 
 	lines, err := repo.Log()
-	c.Assert(err, IsNil)
-	c.Assert(lines, HasLen, 2)
-	c.Assert(lines[0], Matches, "[a-f0-9]{7} blank overwrite")
-	c.Assert(lines[1], Matches, "[a-f0-9]{7} im in ur repo committin ur files")
+	c.Assert(err, gc.IsNil)
+	c.Assert(lines, gc.HasLen, 2)
+	c.Assert(lines[0], gc.Matches, "[a-f0-9]{7} blank overwrite")
+	c.Assert(lines[1], gc.Matches, "[a-f0-9]{7} im in ur repo committin ur files")
 }
 
-func (s *GitDirSuite) TestConflictRevert(c *C) {
+func (s *GitDirSuite) TestConflictRevert(c *gc.C) {
 	source := newRepo(c)
 	updated, err := source.Clone(c.MkDir())
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = ioutil.WriteFile(filepath.Join(updated.Path(), "some-dir"), []byte("hello"), 0644)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = updated.Snapshotf("potential conflict src")
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	conflicted, err := updated.Conflicted()
-	c.Assert(err, IsNil)
-	c.Assert(conflicted, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(conflicted, jc.IsFalse)
 
 	target := charm.NewGitDir(c.MkDir())
 	err = target.Init()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = target.Pull(source)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = ioutil.WriteFile(filepath.Join(target.Path(), "some-dir", "conflicting-file"), []byte("hello"), 0644)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = target.Snapshotf("potential conflict dst")
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	conflicted, err = target.Conflicted()
-	c.Assert(err, IsNil)
-	c.Assert(conflicted, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(conflicted, jc.IsFalse)
 
 	err = target.Pull(updated)
-	c.Assert(err, Equals, charm.ErrConflict)
+	c.Assert(err, gc.Equals, charm.ErrConflict)
 	conflicted, err = target.Conflicted()
-	c.Assert(err, IsNil)
-	c.Assert(conflicted, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(conflicted, jc.IsTrue)
 	dirty, err := target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, true)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsTrue)
 
 	err = target.Revert()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	conflicted, err = target.Conflicted()
-	c.Assert(err, IsNil)
-	c.Assert(conflicted, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(conflicted, jc.IsFalse)
 	dirty, err = target.Dirty()
-	c.Assert(err, IsNil)
-	c.Assert(dirty, Equals, false)
+	c.Assert(err, gc.IsNil)
+	c.Assert(dirty, jc.IsFalse)
 }
 
-func newRepo(c *C) *charm.GitDir {
+func newRepo(c *gc.C) *charm.GitDir {
 	repo := charm.NewGitDir(c.MkDir())
 	err := repo.Init()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = os.Mkdir(filepath.Join(repo.Path(), "some-dir"), 0755)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = ioutil.WriteFile(filepath.Join(repo.Path(), "some-file"), []byte("hello"), 0644)
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = repo.AddAll()
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	err = repo.Commitf("im in ur repo committin ur %s", "files")
-	c.Assert(err, IsNil)
+	c.Assert(err, gc.IsNil)
 	return repo
 }