-
Notifications
You must be signed in to change notification settings - Fork 60
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 typescript types and onboarding ping #525
Conversation
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.
I'm happy with the code changes here, we just need to update the type definitions to match before we can look to ship this. Nice one 🚀
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.
Happy with the changes 🚀
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.
Primary concern is we need to get the tests to pass before releasing anything. Also check some minor typos and questions please: :-)
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.
Ok for prerelease and thus, beta-testing on our systems
src/raygun.loader.js
Outdated
// Request failed | ||
metadata.ping.failedPings++; | ||
} | ||
}) |
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.
This is a bit ugly here, we normally put the .catch in the line. No worries for now
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.
Again, all good for prerelease and soaking in our systems
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.
Couple of comments.
bower.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "raygun4js", | |||
"version": "2.28.0", | |||
"version": "3.1.1", |
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.
@Hamish-taylor Update this please (as you suggested)
package.json
Outdated
], | ||
"title": "Raygun4js", | ||
"description": "Raygun.com plugin for JavaScript", | ||
"version": "3.0.1", | ||
"version": "3.1.1", |
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.
@Hamish-taylor Update here too :)
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.
Looks good to me @Hamish-taylor - just updates on the version numbers.
Co-authored-by: Jasen Palmer <[email protected]>
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.
Ok, follow the usual prelease/soaking process please
No description provided.