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

improve the setup description #45

Merged
merged 31 commits into from
Jun 7, 2019
Merged

improve the setup description #45

merged 31 commits into from
Jun 7, 2019

Conversation

vanesa
Copy link
Contributor

@vanesa vanesa commented May 30, 2019

Improve the setup description for the Sentry extension to ensure better user experience.
Also solves these issues:

@vanesa vanesa requested a review from ryan-blunden May 30, 2019 19:00
@codecov-io
Copy link

codecov-io commented May 30, 2019

Codecov Report

Merging #45 into master will decrease coverage by 0.98%.
The diff coverage is 97.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   85.83%   84.84%   -0.99%     
==========================================
  Files           3        3              
  Lines         120      132      +12     
  Branches       28       35       +7     
==========================================
+ Hits          103      112       +9     
- Misses         17       20       +3
Impacted Files Coverage Δ
src/settings.ts 100% <ø> (ø) ⬆️
src/extension.ts 75.67% <100%> (-0.04%) ⬇️
src/handler.ts 96.42% <96.42%> (-3.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2214e9...5fb0e16. Read the comment docs.

@vanesa vanesa changed the title correct filters config setup description improve the setup description May 31, 2019
@vanesa vanesa requested review from lguychard and chrismwendt June 5, 2019 19:07
Copy link
Contributor

@chrismwendt chrismwendt left a comment

Choose a reason for hiding this comment

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

Approving because I didn't find any logic bugs and none of my comments are blocking.

  • I'm not sure why there's both a package-lock.json and yarn.lock, but I'd git rm package-lock.json
  • I'm not sure why there are renovate commits in this branch. Maybe you merged master into this branch at some point? If you git rebase master from the command line instead, you won't end up with such commits here. Not that it's a problem, it's just a minor distraction during review.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Show resolved Hide resolved
src/test/handler.test.ts Outdated Show resolved Hide resolved
src/handler.ts Outdated Show resolved Hide resolved
src/handler.ts Show resolved Hide resolved
src/handler.ts Show resolved Hide resolved
Copy link

@lguychard lguychard left a comment

Choose a reason for hiding this comment

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

I wish this only included setup description improvements, and not also escaping fixes + various code/test fixes. In the context of this PR ("improve setup description"), consider my comments on docs blocking, and please address comments on code changes in smaller, atomic follow-up PRs.

In general, stick to the one PR - one concern doctrine (or SRPRP, "Single Responsibility Pull Request Principle" - which I totally didn't just make up).

src/test/handler.test.ts Show resolved Hide resolved
src/handler.ts Outdated Show resolved Hide resolved
src/handler.ts Outdated Show resolved Hide resolved
src/handler.ts Outdated Show resolved Hide resolved
src/handler.ts Show resolved Hide resolved
src/handler.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@vanesa vanesa mentioned this pull request Jun 7, 2019
@vanesa vanesa requested a review from lguychard June 7, 2019 02:16
README.md Outdated Show resolved Hide resolved
Co-Authored-By: Loic Guychard <[email protected]>
@vanesa vanesa merged commit 9a5d8dc into master Jun 7, 2019
@vanesa vanesa deleted the vo/fix-readme branch June 7, 2019 16:58
- Python
- Java
- TypeScript, JavaScript ( e.g. `throw new Error()`, `console.error()`)
- Go ( e.g. `err.message()`)
Copy link
Contributor

Choose a reason for hiding this comment

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

I've never seen this code in Go. Do you mean something like error.New()?


File patterns can also be narrowed down to certain folders by specifying this in the RegExp:
Some organizations have multiple Sentry projects that capture errors from various repositories within their organization. Inside each Sentry project, use repository `filters` like so:

```
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this json for syntax highlighting (other codeblocks too)

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.

7 participants