Skip to content

relay: Next iteration of refactoring - #2365

Merged
tonyo merged 20 commits into
masterfrom
fix/relay-more-fixes-after-feedback
Sep 30, 2020
Merged

relay: Next iteration of refactoring#2365
tonyo merged 20 commits into
masterfrom
fix/relay-more-fixes-after-feedback

Conversation

@tonyo

@tonyotonyo commented Sep 25, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR addresses most of the feedback received after testing sessions.

Dockerhub README was updated to provide only bare minimum (links to the docs and Dockerfile): https://hub.docker.com/repository/docker/getsentry/relay

TODO:
* Operating guidelines, add a section with the top options to tweak.

@vercel

vercelBot commented Sep 25, 2020•
edited
Loading

Copy link
Copy Markdown

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sentry/sentry-docs/5hqtnjxkr/sentry.dev
✅ Preview: https://sentry-docs-git-fix-relay-more-fixes-after-feedback.sentry.dev

jan-auer
jan-auer approved these changes Sep 25, 2020
View reviewed changes

@jan-auerjan-auer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. A bunch of smaller suggestions below.

Comment threadsrc/docs/product/relay/getting-started.mdx Outdated
Comment threadsrc/docs/product/relay/getting-started.mdx
Comment threadsrc/docs/product/relay/getting-started.mdx Outdated
Comment threadsrc/docs/product/relay/getting-started.mdx Outdated
Comment threadsrc/docs/product/relay/getting-started.mdx Outdated
```
<code>
<strong><i>http</i></strong>://12345abcdef10111213141516171819@<strong><i>localhost:3000</i></strong>/2345
</code>

@jan-auerjan-auerSep 25, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Ideally, we could somehow indent this and the previous DSN.

@tonyotonyoSep 28, 2020

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Could you explain?

@jan-auerjan-auerSep 28, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The current rendering renders this in a paragraph, but it looks just like a line of text that happens to be code. I was wondering if we could somehow indent this paragraph or do something so this sticks out in the reading flow. This is purely visual.

Comment threadsrc/docs/product/relay/index.mdx Outdated
Comment threadsrc/docs/product/relay/logging.mdx
Comment threadsrc/docs/product/relay/logging.mdx Outdated
Comment threadsrc/docs/product/relay/metrics.mdx Outdated
Comment threadsrc/docs/product/relay/metrics.mdx Outdated
@tonyo@jan-auer
Apply suggestions from code review
52c83e2
Co-authored-by: Jan Michael Auer <mail@jauer.org>
@tonyo

tonyo commented Sep 28, 2020

Copy link
Copy Markdown
ContributorAuthor

@jan-auer Thanks for the suggestions, updated

@vercel
vercelBot temporarily deployed to Preview September 28, 2020 15:22 Inactive
@jan-auer

jan-auer commented Sep 28, 2020

Copy link
Copy Markdown
Member

Added a section on endpoints

image

@tonyo

tonyo commented Sep 28, 2020

Copy link
Copy Markdown
ContributorAuthor

@PeloWriter This is finally ready for another review round 🙏

@tonyo
tonyoforce-pushed the fix/relay-more-fixes-after-feedback branch from 6d96e6e to a8f58fcCompareSeptember 28, 2020 16:09
PeloWriter
PeloWriter approved these changes Sep 28, 2020
View reviewed changes

@PeloWriterPeloWriter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A bit of smoothing and one area of clarification. Thanks.

Comment threadsrc/docs/product/relay/getting-started.mdx Outdated
Comment threadsrc/docs/product/relay/monitoring/index.mdx Outdated
Comment threadsrc/docs/product/relay/monitoring/index.mdx Outdated
Comment threadsrc/docs/product/relay/monitoring/index.mdx Outdated
Comment threadsrc/docs/product/relay/operating-guidelines.mdx Outdated
Comment threadsrc/docs/product/relay/operating-guidelines.mdx Outdated
Comment threadsrc/docs/product/relay/operating-guidelines.mdx Outdated
Comment threadsrc/docs/product/relay/operating-guidelines.mdx Outdated
Comment threadsrc/docs/product/relay/operating-guidelines.mdx Outdated
Comment threadsrc/docs/product/relay/operating-guidelines.mdx Outdated
@tonyo@PeloWriter
Apply suggestions from code review
cb22198
Co-authored-by: Fiona <61481573+PeloWriter@users.noreply.github.com>
@tonyo

tonyo commented Sep 30, 2020

Copy link
Copy Markdown
ContributorAuthor

@PeloWriter Could you check the (last) two small updates, please?

@vercel
vercelBot temporarily deployed to Preview September 30, 2020 17:04 Inactive
@tonyo
tonyo merged commit 40c5ab9 into masterSep 30, 2020
@tonyo
tonyo deleted the fix/relay-more-fixes-after-feedback branch September 30, 2020 17:23
ajjindal pushed a commit that referenced this pull request Sep 30, 2020
@tonyoAJ
relay: Next iteration of refactoring (#2365)
ab4c50b
NisanthanNanthakumar pushed a commit that referenced this pull request Oct 1, 2020
@tonyoNisanthan Nanthakumar
relay: Next iteration of refactoring (#2365)
86c4c82
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Dec 16, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jan-auerjan-auerjan-auer approved these changes
+1 more reviewer
@PeloWriterPeloWriterPeloWriter approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tonyo@jan-auer@PeloWriter