Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Simplify gometalinter ci test #462

Merged
merged 1 commit into from
Aug 23, 2016
Merged

Simplify gometalinter ci test #462

merged 1 commit into from
Aug 23, 2016

Conversation

drewkerrigan
Copy link
Contributor

@drewkerrigan drewkerrigan commented Aug 23, 2016

  • Fixed several unhandled errors warnings
  • Modified gometalinter check in circle.yml to ignore intentional unsafe package usage as well as an SSL warnings.

@jdef jdef added the WIP label Aug 23, 2016
@sargun
Copy link
Contributor

sargun commented Aug 23, 2016

Can we re-enable the tests? And figure out how to solve them rather than just dropping them all? Quite a few of the other linter checks make quite a lot of sense to keep on.

@jdef
Copy link
Contributor

jdef commented Aug 23, 2016

agree with @sargun ; disable-all is a pretty heavy hammer. let's either fix the problems, or put specific disable patterns in there. for example:

gometalinter \
  --vendor --disable=gotype \
  --exclude='cyclomatic complexity .* of function .dnsCharTable.* is high' \
  --concurrency=6 --cyclo-over=12 --tests --deadline=300s ./...

@drewkerrigan
Copy link
Contributor Author

Yes I agree that's a better approach, I'll modify the check

@drewkerrigan
Copy link
Contributor Author

CC: @alberts

@jdef
Copy link
Contributor

jdef commented Aug 23, 2016

Would be really nice if we could put all the metalinter excludes in one place, vs having a copy in Jenkinsfile and in the circleci file. If we want to tackle that in a separate PR that's fine, just want to see it in the backlog somewhere if we're not tackling it here

@jdef
Copy link
Contributor

jdef commented Aug 23, 2016

lgtm once tests pass

@sargun
Copy link
Contributor

sargun commented Aug 23, 2016

@drewkerrigan Can you squash your commits and have a single commit with the summary of the changes, and then I'll merge.

simplifying gometalinter ci test

improve gometalinter usage, fix warnings

put gometalinter check on single line

Adding a placeholder Jenkinsfile

simplify TLSConfig, remove placeholder Jenkinsfile, update circle.yml
@sargun
Copy link
Contributor

sargun commented Aug 23, 2016

LGTM.

@sargun sargun merged commit 7959639 into master Aug 23, 2016
@jdef jdef removed the WIP label Aug 23, 2016
@sargun sargun deleted the ack-lint-fixes branch August 23, 2016 22:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants