I'd like gccgo to pass reliably, but I think it falls under a CI test rather than a pre-commit test. (Because otherwise it roughly doubles the time it takes to know whether your patch will land.)
I don't have specific feedback on the behavior differences, but the changes seem sane to me. An errors library is certainly something that I'd like to see shared between code bases. John =:-> On Wed, May 14, 2014 at 1:24 PM, Tim Penhey <tim.pen...@canonical.com>wrote: > Hi all, > > I took it upon myself to get Rog's errgo library used inside juju-core. > > Dimiter recently did a hunk of work in the juju-core/errors package to > have functions to add context to some core error types while keeping > their type. > > What I did was to move this errors package out of juju-core and into its > own package, and update it to use the errgo as a base. For those that > are interested the diff for the errors_test (to show that everything > works as before) is as shown at the bottom of this email. > > I also added in simple methods based on my earlier error handling > package, but changed them to be based on errgo. This adds in the > functions: New, Errorf, Check, Trace, Annotate, Annotatef, Wrap, > ErrorStack. > > One key difference in behaviour between these methods and the base errgo > ones is that the cause is always passed through. Unless you call Wrap, > in which case the cause changes, but that is the purpose of the call. > > I have a branch of juju-core up as work in progress that does the > following: > * removes the dependency on github.com/errgo/errgo > * changes uses of errgo/errgo to juju/errors > * adds both juju/errgo and juju/errors to the dependency file > * removes the juju-core/errors package > * changes all the juju-core/errors import statement to use > github.com/juju/errors (and puts the import statement in the > right block) > https://code.launchpad.net/~thumper/juju-core/juju-errors/+merge/219311 > 3602 lines (+252/-658) 156 files modified > > I have tested juju/errors with both gc and gccgo and the tests pass with > both. Interestingly juju/errgo has a few test failures with gccgo, but > Dave is looking at them and assures me they are simple fixes. > As an aside, I do think that any new dependencies we add should pass all > tests with both gc and gccgo. > > Cheers, > > Tim > > $ diff -u ~/src/juju-core/trunk/errors/errors_test.go errors_test.go > --- /home/tim/src/juju-core/trunk/errors/errors_test.go 2014-05-14 > 19:18:53.376121000 +1200 > +++ errors_test.go 2014-05-13 17:14:50.329051637 +1200 > @@ -1,5 +1,5 @@ > -// Copyright 2013 Canonical Ltd. > -// Licensed under the AGPLv3, see LICENCE file for details. > +// Copyright 2013, 2014 Canonical Ltd. > +// Licensed under the GPLv3, see LICENCE file for details. > > package errors_test > > @@ -8,12 +8,10 @@ > "fmt" > "reflect" > "runtime" > - "testing" > > + "github.com/juju/errors" > jc "github.com/juju/testing/checkers" > gc "launchpad.net/gocheck" > - > - "launchpad.net/juju-core/errors" > ) > > // errorInfo holds information about a single error type: a satisfier > @@ -40,10 +38,6 @@ > > var _ = gc.Suite(&errorsSuite{}) > > -func Test(t *testing.T) { > - gc.TestingT(t) > -} > - > func (t *errorInfo) satisfierName() string { > value := reflect.ValueOf(t.satisfier) > f := runtime.FuncForPC(value.Pointer()) > @@ -175,6 +169,12 @@ > runErrorTests(c, errorTests, true) > } > > +func (*errorsSuite) TestContextfNotNewer(c *gc.C) { > + err := fmt.Errorf("simple error") > + errors.Contextf(&err, "annotate") > + c.Assert(err, gc.ErrorMatches, "annotate: simple error") > +} > + > func (*errorsSuite) TestAllErrors(c *gc.C) { > errorTests := []errorTest{} > for _, errInfo := range allErrors { > > -- > Juju-dev mailing list > Juju-dev@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev >
-- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
