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

update to use /api/{projectId}/store #105

Closed
wants to merge 5 commits into from
Closed

update to use /api/{projectId}/store #105

wants to merge 5 commits into from

Conversation

Raynos
Copy link
Contributor

@Raynos Raynos commented Dec 31, 2014

The default recommended rate limiting on the sentry
documentation rate limits by projectId.

It's a better default to talk to a sentry server using the
/api/{projectId}/store API.

This way multiple services talking to the same sentry
server will not all get rate limited under the same
projectId (which is presumably the empty string).

@@ -18,8 +18,7 @@
"raven": "./bin/raven"
},
"scripts": {
"pretest": "npm install",
"test": "NODE_ENV=test mocha --reporter dot && NODE_ENV=test coffee ./test/run.coffee"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why removing the coffee test run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mattrobenolt it's a duplicate test run. There's no reason to run the tests twice. I can revert that change though since it's not relevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted that change as it was a bikeshed :)

Copy link
Contributor

Choose a reason for hiding this comment

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

It wasn't exactly a duplicate run becuase coffeescript modifies the stack traces and injects stuff in. So we run the entire test suite with the coffee script runner as well to make sure that's ok. :)

@Raynos
Copy link
Contributor Author

Raynos commented Dec 31, 2014

I noticed I forgot to fix some tests after rebasing on master, those are fixed now.

@mattrobenolt
Copy link
Contributor

@Raynos You don't need to worry about node 0.6, the tests have been broken for a while for them because of stupid stuff. See: #100

@Raynos
Copy link
Contributor Author

Raynos commented Dec 31, 2014

@mattrobenolt I removed the node 0.6 from travis for you :)

@dcramer
Copy link
Member

dcramer commented May 12, 2015

That was a surprising pain in the ass to pull in :)

@dcramer dcramer closed this May 12, 2015
@dcramer
Copy link
Member

dcramer commented May 12, 2015

ref b725bd5

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.

None yet

3 participants