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

Revisit a merged PR which broke our Travis CI and the support for Caddy v2 #8

Closed
hsluoyz opened this issue Jan 11, 2021 · 12 comments
Closed
Assignees
Labels

Comments

@hsluoyz
Copy link
Member

hsluoyz commented Jan 11, 2021

See details at: #7 (comment)

@hsluoyz
Copy link
Member Author

hsluoyz commented Jan 11, 2021

@closetool can you work on this?

@kilosonc
Copy link
Contributor

ok, I'll try to fix it

@kilosonc
Copy link
Contributor

I guess the go version in travis.yml should be 1.14.x at least
image
and the coveralls service should be opened

@hsluoyz
Copy link
Member Author

hsluoyz commented Jan 12, 2021

@closetool coveralls is enabled: https://coveralls.io/github/casbin/caddy-authz?branch=master

@kilosonc
Copy link
Contributor

@hsluoyz yes, but some packages require go version over 1.14 like github.com/caddyserver/certmagic

@hsluoyz
Copy link
Member Author

hsluoyz commented Jan 12, 2021

@closetool good to know!

Please make a PR to fix the Go versions here: https://github.com/casbin/caddy-authz/blob/master/.travis.yml#L6

We usually test against several popular Go versions including tip. Please refer to Go Casbin repo for it.

@hsluoyz
Copy link
Member Author

hsluoyz commented Jan 12, 2021

@closetool merged: #9

@hsluoyz
Copy link
Member Author

hsluoyz commented Jan 12, 2021

@closetool good to see CI recovered! But I saw the code coverage has dropped to 0%, can you investigate why?

image

https://coveralls.io/github/casbin/caddy-authz?branch=master

image

@kilosonc
Copy link
Contributor

sure

@kilosonc
Copy link
Contributor

@hsluoyz I compared v2.0.0 and v1.0.2, and I found there's no authz_test.go in v2.0.0
perhaps that's why coverage dropped to 0%

@hsluoyz
Copy link
Member Author

hsluoyz commented Jan 12, 2021

good found! I think this PR: https://github.com/casbin/caddy-authz/pull/7/files deleted authz_test.go. We need to add it back and make it right in the new Caddy v2 scenario, as our code needs testing to make it behave as expected.

image

@kilosonc
Copy link
Contributor

ok, I'll work on it

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

No branches or pull requests

2 participants