Proposed new dependency: github.com/juju/errors (and github.com/juju/errgo)

[John Meinel]

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.penhey at 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 at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20140514/f3cda575/attachment.html>