3 ms·

What random formatting do you mean, other than what go fmt would produce? What unidiomatic naming conventions? I thought I removed all the dead code... which on

by andlabs 12y ago

What random formatting do you mean, other than what go fmt would produce? What unidiomatic naming conventions? I thought I removed all the dead code... which ones do you still see? What suggestions can you make about organization?

DigitalJack 12y ago
lines 9-16 of this file: https://github.com/andlabs/ui/blob/master/checkbox.go https://github.com/andlabs/ui/blob/master/checkbox.go or lines 10-12 of this file: https://github.com/andlabs/ui/blob/master/area_darwin.go https://github.com/andlabs/ui/blob/master/area_darwin.go those were the first to files I picked. I have no comment on organization--something I struggle with.
- andlabs 12y ago
  Are you sure the code in checkbox.go is commented out? That's not what I see. As for area_darwin.go, though it's a comment, it's actually C code being sent to an FFI: http://tip.golang.org/cmd/cgo/ http://tip.golang.org/cmd/cgo/
  - marcus_holmes 12y ago
    Don't sweat it too much. The first thing that happens whenever anyone publishes any Go code on the internet is a random commenter pops up and criticises it for being "unidiomatic". I'm beginning to think that "idiomatic" is actually a portmanteau of "automatic" and "idiotic" because it's such a knee-jerk reaction in the community now. If it's flaws in your variable naming and indentation, let it slide. If it's flaws in the logic or functionality, deal with it.
    - AYBABTME 12y ago
      'idiomatic' is used as newspeak for 'not the style I personally recognize as being the true Go way'.
      - sagichmal 12y ago
        > 'idiomatic' is used as newspeak for 'not the style I > personally recognize as being the true Go way'. There is, in fact, a style which is "the true Go way".
        AYBABTME 12y ago
        'idiomatic' is not an objective term, but people use it like if it were. There's a true Go way, the style used in the stdlib; but even the stdlib is not consistent and you need to pick and choose. Then the standard library doesn't cover all possible cases. My definition of idiomatic Go code is: 1. It passes gofmt 2. It passes govet 3. It passes golint 4. It checks it's errors. 5. It looks like code in the stdlib. 6. For the rest, it looks like code written by other people that I recognize as being good Go developers. 7. For the rest, it looks like my own code. I think that's a stricter definition than what most people think of when they criticize code as being unidiomatic. However it's still very subjective. From 1 to 4 are objective measurements. Point 5 is somewhat subjective. Points 6 and 7 are simply a matter of my personal experience through life and how circumstances led me to encounter a specific subset of code; entirely subjective.
        sagichmal 12y ago
        The definition of idiomatic Go code is go fmt, go vet, golint, and https://code.google.com/p/go-wiki/wiki/CodeReviewComments https://code.google.com/p/go-wiki/wiki/CodeReviewComments.
sagichmal 12y ago
> What random formatting? Primarily here I saw a lot of arbitrary indentation. As far as I can tell, everything would be fixed by a gofmt -w... https://github.com/andlabs/ui/blob/master/button.go#L16-19 https://github.com/andlabs/ui/blob/master/button.go#L16-19 https://github.com/andlabs/ui/blob/master/area_unix.go#L35-36 https://github.com/andlabs/ui/blob/master/area_unix.go#L35-3... https://github.com/andlabs/ui/blob/master/area_unix.go#L107 https://github.com/andlabs/ui/blob/master/area_unix.go#L107 https://github.com/andlabs/ui/blob/master/area_unix.go#L200 https://github.com/andlabs/ui/blob/master/area_unix.go#L200 https://github.com/andlabs/ui/blob/master/area_unix.go#L329-340 https://github.com/andlabs/ui/blob/master/area_unix.go#L329-... https://github.com/andlabs/ui/blob/master/grid.go#L18-26 https://github.com/andlabs/ui/blob/master/grid.go#L18-26 > What unidiomatic naming conventions? Primarily here I saw a lot of underscores, which are never used in pure Go. Some of that maybe is due to Cgo conventions, but some of it isn't. https://github.com/andlabs/ui/blob/master/area_unix.go#L209 https://github.com/andlabs/ui/blob/master/area_unix.go#L209 https://github.com/andlabs/ui/blob/master/sysdata.go#L23 https://github.com/andlabs/ui/blob/master/sysdata.go#L23
- andlabs 12y ago
  All indentation ones except the L35-37 ones are lack of go fmt. I will go fmt once I can double-check exactly what go fmt changes. L35-37 I'm not sure what problem that has; I just put the very long bitmask on its own line. The underscores in our_xxx_xxx_xxx I don't think would be much of an issue. They unintentionally match the GTK+ naming conventions. I don't know why I did them, but I don't consider it a world-ending issue worth fixing. Same for _xSysData, but in that case the _ is there in case I accidentally said xSysData somewhere else.

by andlabs 12y ago

DigitalJack 12y ago

lines 9-16 of this file: https://github.com/andlabs/ui/blob/master/checkbox.go https://github.com/andlabs/ui/blob/master/checkbox.go or lines 10-12 of this file: https://github.com/andlabs/ui/blob/master/area_darwin.go https://github.com/andlabs/ui/blob/master/area_darwin.go those were the first to files I picked. I have no comment on organization--something I struggle with.

andlabs 12y ago
Are you sure the code in checkbox.go is commented out? That's not what I see. As for area_darwin.go, though it's a comment, it's actually C code being sent to an FFI: http://tip.golang.org/cmd/cgo/ http://tip.golang.org/cmd/cgo/
- marcus_holmes 12y ago
  Don't sweat it too much. The first thing that happens whenever anyone publishes any Go code on the internet is a random commenter pops up and criticises it for being "unidiomatic". I'm beginning to think that "idiomatic" is actually a portmanteau of "automatic" and "idiotic" because it's such a knee-jerk reaction in the community now. If it's flaws in your variable naming and indentation, let it slide. If it's flaws in the logic or functionality, deal with it.
  - AYBABTME 12y ago
    'idiomatic' is used as newspeak for 'not the style I personally recognize as being the true Go way'.
    - sagichmal 12y ago
      > 'idiomatic' is used as newspeak for 'not the style I > personally recognize as being the true Go way'. There is, in fact, a style which is "the true Go way".
      - AYBABTME 12y ago
        'idiomatic' is not an objective term, but people use it like if it were. There's a true Go way, the style used in the stdlib; but even the stdlib is not consistent and you need to pick and choose. Then the standard library doesn't cover all possible cases. My definition of idiomatic Go code is: 1. It passes gofmt 2. It passes govet 3. It passes golint 4. It checks it's errors. 5. It looks like code in the stdlib. 6. For the rest, it looks like code written by other people that I recognize as being good Go developers. 7. For the rest, it looks like my own code. I think that's a stricter definition than what most people think of when they criticize code as being unidiomatic. However it's still very subjective. From 1 to 4 are objective measurements. Point 5 is somewhat subjective. Points 6 and 7 are simply a matter of my personal experience through life and how circumstances led me to encounter a specific subset of code; entirely subjective.
        sagichmal 12y ago
        The definition of idiomatic Go code is go fmt, go vet, golint, and https://code.google.com/p/go-wiki/wiki/CodeReviewComments https://code.google.com/p/go-wiki/wiki/CodeReviewComments.

sagichmal 12y ago

> What random formatting? Primarily here I saw a lot of arbitrary indentation. As far as I can tell, everything would be fixed by a gofmt -w... https://github.com/andlabs/ui/blob/master/button.go#L16-19 https://github.com/andlabs/ui/blob/master/button.go#L16-19 https://github.com/andlabs/ui/blob/master/area_unix.go#L35-36 https://github.com/andlabs/ui/blob/master/area_unix.go#L35-3... https://github.com/andlabs/ui/blob/master/area_unix.go#L107 https://github.com/andlabs/ui/blob/master/area_unix.go#L107 https://github.com/andlabs/ui/blob/master/area_unix.go#L200 https://github.com/andlabs/ui/blob/master/area_unix.go#L200 https://github.com/andlabs/ui/blob/master/area_unix.go#L329-340 https://github.com/andlabs/ui/blob/master/area_unix.go#L329-... https://github.com/andlabs/ui/blob/master/grid.go#L18-26 https://github.com/andlabs/ui/blob/master/grid.go#L18-26 > What unidiomatic naming conventions? Primarily here I saw a lot of underscores, which are never used in pure Go. Some of that maybe is due to Cgo conventions, but some of it isn't. https://github.com/andlabs/ui/blob/master/area_unix.go#L209 https://github.com/andlabs/ui/blob/master/area_unix.go#L209 https://github.com/andlabs/ui/blob/master/sysdata.go#L23 https://github.com/andlabs/ui/blob/master/sysdata.go#L23

andlabs 12y ago
All indentation ones except the L35-37 ones are lack of go fmt. I will go fmt once I can double-check exactly what go fmt changes. L35-37 I'm not sure what problem that has; I just put the very long bitmask on its own line. The underscores in our_xxx_xxx_xxx I don't think would be much of an issue. They unintentionally match the GTK+ naming conventions. I don't know why I did them, but I don't consider it a world-ending issue worth fixing. Same for _xSysData, but in that case the _ is there in case I accidentally said xSysData somewhere else.