dominikh / go-js-dom

MIT License
252 stars 42 forks source link

add canvas api support #2

Closed josedonizetti closed 10 years ago

josedonizetti commented 10 years ago

There a few methods/properties missing. Which I will add later. But a lot can be done with the ones supported already.

dominikh commented 10 years ago

Thanks.

A number of stylistic issues first:

  1. Please run gofmt on the code
  2. Please write comments like // comment here, not //comment here
  3. For the comments that act as headers (e.g. the one in line 1776), please leave a blank line after them so that godoc doesn't associate them with the functions that follow immediately.
  4. Instead of writing

    if maxWidth == -1 {
       something()
    } else {
       somethingElse()
    }

    write

    if maxWidth == -1 {
       something()
       return
    } 
    somethingElse()

That's as far as global stylistic issues go. Notes to specific lines will follow as inline comments.

dominikh commented 10 years ago

Please don't let the amount of "complaints" discourage you; almost all of them are superficial and just style nitpicks. I didn't spot anything actually wrong :-)

josedonizetti commented 10 years ago

@dominikh Think I fixed all of them. And don't worry about giving feedback, is always good. I'm still learning go, and sometimes forget about the styling rules. Thanks! :)

dominikh commented 10 years ago

Merged as 78f0940, thank you. The only code change that I added is that I changed counterclockwise from int to bool. That didn't warrant another revision of this PR :)