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

add documentation regarding raven-teskit plugin #1231

Merged
merged 4 commits into from
Mar 19, 2018

Conversation

zivl
Copy link
Contributor

@zivl zivl commented Feb 14, 2018

I've written a test-kit for the raven-js library, I think it should be added to the documentation.

I know I looked for something like that A LOT for my development and testing purposes but since such was not exist - I wrote one!
so here it is: https://github.com/wix/raven-testkit

Hope you like it, and will be happy to get your feedback!
Enjoy!
Ziv

@zivl zivl requested a review from kamilogorek as a code owner February 14, 2018 17:19
@kamilogorek
Copy link
Contributor

Nice! I'll try to give it a look this week :) Cheers!

@kamilogorek
Copy link
Contributor

Sorry for the delay @zivl, we're kinda swamped with work now. I'll try to get this done soon.

@kamilogorek kamilogorek self-assigned this Feb 27, 2018
@zivl
Copy link
Contributor Author

zivl commented Mar 13, 2018

@kamilogorek anything new? anything I can help to make it ease with your review?

@@ -11,6 +11,14 @@
"index#reporting-errors"
]
},
"javascript.textkit": {
Copy link
Contributor

Choose a reason for hiding this comment

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

This entry is used for a "Framework selection panel" when creating a new project, so we cannot really add it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

where do you think it suites the most?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh... saw your comment below about the Tips & Tricks


.. code-block:: javascript

const testKit = testKitInitializer(Raven)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd include Raven import here to not confuse users, as right now it somehow just magically appears out of nowhere :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. ok


.. code-block:: javascript

import testKitInitializer from 'raven-testkit'
Copy link
Contributor

Choose a reason for hiding this comment

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

Incorrect indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix

import testKitInitializer from 'raven-testkit'


Then you may create a `testkit` instance and validate your reports against it as follows:
Copy link
Contributor

Choose a reason for hiding this comment

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

Double-ticks has to be used as it's RST, not markdown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix

@@ -40,6 +40,7 @@ To install a plugin just include the plugin **after** Raven has been loaded and
ember
react
vue
raven-testkit
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you aware that it'll only show up here https://docs.sentry.io/clients/javascript/integrations/ and not in the sidebar? I'd not add it to the sidebar anyway, as it's more a helpful tool than framework integration.
Let's maybe add a section in in Tips and Tricks that can mention it and link to the integration page?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or just completely move it to Tips and Tricks section instead of a separate page? As, as I've mentioned before, it's not really an integration :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. will move it there :)

@kamilogorek
Copy link
Contributor

Thanks for your patience @zivl, got completely swamped in other things. Reviewed now :)

@zivl
Copy link
Contributor Author

zivl commented Mar 18, 2018

Hey @kamilogorek,
I've updated the PR. Now the docs is only in Tips and Tricks and I've reduced the amount of text for easier reading along with the Tips and Tricks page.
You're free to review

@kamilogorek kamilogorek merged commit 9b83e9e into getsentry:master Mar 19, 2018
@kamilogorek
Copy link
Contributor

Thanks @zivl! :)

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