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

sound: extend CI checks #14

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

sound: extend CI checks #14

wants to merge 12 commits into from

Conversation

mewmew
Copy link
Member

@mewmew mewmew commented Sep 7, 2018

Add GolangCI meta linter among others.

Code coverage token will be added in a follow-up PR.

@wsc1
Copy link
Member

wsc1 commented Sep 7, 2018

I added ci_checks.sh to meta

@wsc1
Copy link
Member

wsc1 commented Sep 7, 2018

LGTM

@wsc1 wsc1 closed this Sep 7, 2018
@wsc1 wsc1 reopened this Sep 7, 2018
@wsc1
Copy link
Member

wsc1 commented Sep 7, 2018

sorry, didn't mean to close

Copy link
Member

@wsc1 wsc1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why revert ci_checks script?

@mewmew
Copy link
Member Author

mewmew commented Sep 7, 2018

why?

I couldn't get code coverage reporting to work with the token you sent, so had to update and trouble shoot the ci_checks script. And I don't have access to zc/meta as far as I know.

@wsc1
Copy link
Member

wsc1 commented Sep 7, 2018

Github tells me you have admin access on all zc repos.

@mewmew
Copy link
Member Author

mewmew commented Sep 7, 2018

Oh well, we can live without code coverage reporting for now. I tried to trouble shoot why the COVERALLS_TOKEN is not set by travis, but couldn't figure it out. Without this token, this branch is never taken:

if [ "${COVERALLS_TOKEN}" ]; then
	echo "### go tool cover"
	go test -covermode=count -coverprofile=coverage.out ./...
	go tool cover -func coverage.out

	# To submit the test coverage result to coveralls.io use goveralls
	# (https://github.com/mattn/goveralls)
	goveralls -coverprofile coverage.out -service travis-ci -repotoken ${COVERALLS_TOKEN}
fi

@mewmew
Copy link
Member Author

mewmew commented Sep 7, 2018

Github tells me you have admin access on all zc repos.

Oh, that's more than I know. I thought I was only part of ext.

@mewmew
Copy link
Member Author

mewmew commented Sep 7, 2018

Feel free to merge this and then the COVERALLS_TOKEN issue can be trouble shot later.

got to go.

Cheers,
/u

@wsc1
Copy link
Member

wsc1 commented Sep 8, 2018

Since we've got so many CI build issues, maybe I'll clean up the various more cosmetic issues from these new CI checks in the repo first...

@wsc1
Copy link
Member

wsc1 commented Sep 8, 2018

Looking more closely, it seems the linter is checking whether error return values are checked?

I don't like that; for example, when using sndbuf there are no io errors but it has them to fit the interface. Is there some way to refine the linter checks or the files on which they operate? Don't want to lose a badge over something like that :(

@wsc1
Copy link
Member

wsc1 commented Sep 27, 2018

any ideas on how to make travis recognise stdatomic.h via cgo?

@mewmew
Copy link
Member Author

mewmew commented Sep 27, 2018

It should work with newer versions of GCC, as suggested in cmus/cmus#604

If there is something specific to Cgo that prevents it from working, I don't know.

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

Successfully merging this pull request may close these issues.

None yet

2 participants