code review 10457043: go.talks/pkg/present: fix a couple of vet-detected issues

go.talks/pkg/present: fix a couple of vet-detected issues

[r, 2013-06-21 17:10:07]

Hello adg (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.talks

[bradfitz, 2013-06-21 17:11:51]

LGTM

But not all shadowing is bad.  In a small function like that where you can
see the whole thing, it's not incredibly offensive that s is shadowed.  But
yes, it could bite in the future.



On Fri, Jun 21, 2013 at 10:10 AM, <r@golang.org> wrote:

> Reviewers: adg,
>
> Message:
> Hello adg (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go.**talks <https://code.google.com/p/go.talks>
>
>
> Description:
> go.talks/pkg/present: fix a couple of vet-detected issues
>
> Please review this at
https://codereview.appspot.**com/10457043/<https://codereview.appspot.com/104...
>
> Affected files:
>   M pkg/present/code.go
>   M pkg/present/parse.go
>
>
> Index: pkg/present/code.go
> ==============================**==============================**=======
> --- a/pkg/present/code.go
> +++ b/pkg/present/code.go
> @@ -157,7 +157,7 @@
>         switch n := arg.(type) {
>         case int:
>                 if n <= 0 || n > max {
> -                       return 0, "", false, fmt.Errorf("%%d is out of
> range", n)
> +                       return 0, "", false, fmt.Errorf("%d is out of
> range", n)
>                 }
>                 return n, "", true, nil
>         case string:
> Index: pkg/present/parse.go
> ==============================**==============================**=======
> --- a/pkg/present/parse.go
> +++ b/pkg/present/parse.go
> @@ -90,8 +90,8 @@
>
>  func (s Section) Sections() (sections []Section) {
>         for _, e := range s.Elem {
> -               if s, ok := e.(Section); ok {
> -                       sections = append(sections, s)
> +               if section, ok := e.(Section); ok {
> +                       sections = append(sections, section)
>                 }
>         }
>         return
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[r, 2013-06-21 17:48:02]

*** Submitted as
https://code.google.com/p/go/source/detail?r=78b8854bfdf3&repo=talks ***

go.talks/pkg/present: fix a couple of vet-detected issues

R=adg, bradfitz
CC=golang-dev
https://codereview.appspot.com/10457043

[r, 2013-06-21 17:48:53]

Agree, but I think shadowing the receiver like this is questionable style.