-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add CI and make tests externally runnable #7
Conversation
|
||
declare global { | ||
// eslint-disable-next-line no-var | ||
var testClient: CortexClient; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not import this where needed? This will pollute the global scope for non-test code as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment about global scope, LGTM o/w.
@jmoseley will follow up in another PR doing this via an import. Want to lock in CI now before we start making more changes to the SDK today. But agree this is a good point and I will update accordingly |
Fixes #1
test
test:fast
andtest:ci
targets:dev
and:prod
targets - these are for us only, and external contributors won't be able to run these scripts, but want to maintain them for convenienceNote: had to disable the list content tests due to some bugs that I found in the API. I believe we're following up on that tomorrow.