Skip to content

Javascript Error Tracking#134

Closed
MikeRogers0 wants to merge 23 commits intoMindscapeHQ:masterfrom
MikeRogers0:feature/js-error-tracking
Closed

Javascript Error Tracking#134
MikeRogers0 wants to merge 23 commits intoMindscapeHQ:masterfrom
MikeRogers0:feature/js-error-tracking

Conversation

@MikeRogers0
Copy link
Contributor

@MikeRogers0 MikeRogers0 commented Sep 23, 2018

This adds the js_api_key configuration option, if it's present it'll inject the JS code to a HTML page.

Some key changes:

  • I moved the folder specs to spec to be more inline with other projects I've worked on.
  • I added a dummy app for full integration tests
  • I changed the RSpec syntax to use expect( something ).to eq( something else ) as it's closer to their sample docs.
  • I upgraded a few of the gems to be the latest.
  • I added SimpleCov to track coverage of RSpec

This is for the issue #132

@UberMouse
Copy link
Contributor

Thanks for this Mike. I'll take a look when I get time, either this week or the next.

@MikeRogers0
Copy link
Contributor Author

@UberMouse Thank you for reviewing :) It's a big change, so take your time & enjoy it.

Copy link
Contributor

@UberMouse UberMouse left a comment

Choose a reason for hiding this comment

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

Sorry for the lengthy delay in getting around to this @MikeRogers0! It's looking good, just a few changes I would like around the tests and to include the user/version information.

@MikeRogers0
Copy link
Contributor Author

Awesome thank you for the review, I'll aim to get on it possibly next weekend :)

@MikeRogers0
Copy link
Contributor Author

Eak my weekend got away from me a bit - don't worry, I've not forgotten about this :)

@MikeRogers0 MikeRogers0 changed the title [WIP] Javascript Error Tracking Javascript Error Tracking Dec 26, 2018
@MikeRogers0
Copy link
Contributor Author

Thanks for your patience @UberMouse 👍

I got a bit of time this evening to do the updated you requested (and respond to the other comments). Any other changes (I might have some time over NYE)?

Copy link
Contributor

@UberMouse UberMouse left a comment

Choose a reason for hiding this comment

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

Looking good :) just a few changes

end

def js_api_version
@js_api_version ||= Raygun.configuration.js_api_version || '1.14.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should default to '' so that it always falls back to the latest version unless you specifically set it to another version. The url should be fine if it ends up like //cdn.raygun.io/raygun4js//raygun.min.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@UberMouse Updated :D

@UberMouse
Copy link
Contributor

Thanks for that Mike. I tested it and that URL format with double slashes in it doesn't work on S3 so I've just gone ahead and fixed that part up since it was my suggestion. I've opened a new PR and will merge that work in shortly over there.

@UberMouse UberMouse closed this Jan 15, 2019
@MikeRogers0 MikeRogers0 deleted the feature/js-error-tracking branch January 15, 2019 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants