Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Golint issues and CI #1

Closed
mewmew opened this issue Aug 7, 2018 · 6 comments
Closed

Golint issues and CI #1

mewmew opened this issue Aug 7, 2018 · 6 comments
Labels
umbrella applies to all projects

Comments

@mewmew
Copy link
Member

mewmew commented Aug 7, 2018

Issues reported by golint, excluding missing documentation.

u@x61s ~/g/s/z/sound> golint ./... | grep -v "exported"
cae.go:20:5: error var ChannelAlignmentError should have name of the form ErrFoo
duplex.go:37:5: error var FrameAlignmentError should have name of the form ErrFoo
duplex.go:37:38: error strings should not be capitalized or end with punctuation or a newline
freq/i.go:25:9: if block ends with a return statement, so drop this else and outdent its block
freq/t.go:41:10: if block ends with a return statement, so drop this else and outdent its block
freq/t.go:139:1: receiver name s should be consistent with previous receiver name f for T
ops/copy.go:26:21: error strings should not be capitalized or end with punctuation or a newline
ops/join.go:75:26: error strings should not be capitalized or end with punctuation or a newline
ops/mix.go:15:26: error strings should not be capitalized or end with punctuation or a newline

Preferably, Travis-CI could be configured for each project to do these checks, and others.

For an example configuration file, see https://github.com/mewmew/mips/blob/master/.travis.yml which handles:

  • build status
  • test case status
  • race detection status
  • golint status
  • code coverage update via Coveralls
  • GolangCI metalinter status
  • CodeClimate status
@wsc1 wsc1 added the umbrella applies to all projects label Aug 7, 2018
@wsc1
Copy link
Member

wsc1 commented Aug 7, 2018

Yes, all this is being worked on, CI needs to be set up. It's a bit trickier to do than other projects I'm aware of for the sio to run tests (any ideas there?)

I haven't used CodeClimate before, and it looks commercial to me. We will add at least builds, tests, race where appropriate. The others are more cosmetic and I've no experience with CodeClimate to get a feel of what it's worth, so let's wait a bit for them to see.

@mewmew
Copy link
Member Author

mewmew commented Aug 7, 2018

Yes, all this is being worked on, CI needs to be set up. It's a bit trickier to do than other projects I'm aware of for the sio to run tests (any ideas there?)

Running the tests on Linux should be rather straight forward on Travis CI. Just apt-get the alsa dependencies if needed, and then go test as usual. For an example on installing dependencies on Travis, see https://github.com/mewspring/e/blob/master/.travis.yml

With regards to testing on Mac, I can't offer any help. Sorry.

I haven't used CodeClimate before, and it looks commercial to me. We will add at least builds, tests, race where appropriate. The others are more cosmetic and I've no experience with CodeClimate to get a feel of what it's worth, so let's wait a bit for them to see.

Haha., yeah. Skip CodeClimate. It definitely feels commercial. I only recently tried using it, and it did report some places in the code that could be cleaned up. But GolangCI metalinter is good too, and open source for that matter.

What I'd recommend is:

  • build status: go install
  • test status: go test
  • test coverage: Coveralls.io (also drone.io and others, drone required more permissions on GitHub so I decided to use Coveralls for my personal projects.)
  • race detection: go test -race, especially since sound.Pipe permits concurrent use.
  • golint; and GolangCI metalinter.

For a quick howto on using adding Coveralls test coverage for Travis CI: see decomp/decomp#4 (comment)

Edit: The goclean.sh script runs the following tasks. Feel free to adopt it in whichever way to suit your needs.

# The script does automatic checking on a Go package and its sub-packages, including:
#    1. gofmt         (http://golang.org/cmd/gofmt/)
#    2. goimports     (http://godoc.org/golang.org/x/tools/cmd/goimports)
#    3. golint        (https://github.com/golang/lint)
#    4. go vet        (http://golang.org/cmd/vet)
#    5. race detector (http://blog.golang.org/race-detector)
#    6. test coverage (http://blog.golang.org/cover)

@wsc1
Copy link
Member

wsc1 commented Aug 7, 2018

Thanks for the pointers

For questions regarding testing sio, I was referring the to the output being actually listened to and sound actually generated for capture. I have no idea how cloud CI might do that. I am not sure what sound devices ALSA will find either.

@mewmew
Copy link
Member Author

mewmew commented Aug 7, 2018

For questions regarding testing sio, I was referring the to the output being actually listened to and sound actually generated for capture. I have no idea how cloud CI might do that. I am not sure what sound devices ALSA will find either.

Haha, yeah. That is a good point, seems quite difficult to test audio output; as if you emulate the output device, then perhaps the emulator works but the drivers (or libraries) for the real hardware doesn't.

Sorry for the misunderstanding :)

@wsc1
Copy link
Member

wsc1 commented Aug 9, 2018

Closing as at least basic travis-ci is set up and lint cleanups incorporated. We can refine the CI over time.

@wsc1 wsc1 closed this as completed Aug 9, 2018
@mewmew
Copy link
Member Author

mewmew commented Aug 10, 2018

Closing as at least basic travis-ci is set up and lint cleanups incorporated. We can refine the CI over time.

Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
umbrella applies to all projects
Projects
None yet
Development

No branches or pull requests

2 participants