Skip to content

Java: Add BOM docs. - #3832

Merged
bruno-garcia merged 8 commits into
getsentry:masterfrom
maciejwalkowiak:gh-3652
Jul 21, 2021
Merged

Java: Add BOM docs.#3832
bruno-garcia merged 8 commits into
getsentry:masterfrom
maciejwalkowiak:gh-3652

Conversation

@maciejwalkowiak

@maciejwalkowiak maciejwalkowiak commented Jul 5, 2021

Copy link
Copy Markdown
Contributor

Closes #3652

@vercel

vercel Bot commented Jul 5, 2021

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/CCjxxms1pHFDJhE56xjN3eB1LGy6
✅ Preview: https://sentry-docs-git-fork-maciejwalkowiak-gh-3652.sentry.dev

Comment thread src/platforms/common/index.mdx Outdated

<PlatformContent includePath="getting-started-install" />

<PlatformContent includePath="getting-started-install-bom" supported={["java"]} />

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Perhaps there is a better way to include this piece into every Java guide?

@maciejwalkowiak

Copy link
Copy Markdown
Contributor Author

cc @marandaneto @bruno-garcia

@maciejwalkowiak

Copy link
Copy Markdown
Contributor Author

I am not 100% convinced that this is the right place to put BOM docs. AFAIR we aimed "Getting Started" for having only essential information - BOM docs are useful only if more than one Sentry dependency is used (Android + Coroutines, Spring + Logback etc).

@marandaneto

Copy link
Copy Markdown
Contributor

I am not 100% convinced that this is the right place to put BOM docs. AFAIR we aimed "Getting Started" for having only essential information - BOM docs are useful only if more than one Sentry dependency is used (Android + Coroutines, Spring + Logback etc).

based on my comment here #3652 (comment)
I'd say its best to be within the install step, so here most of the cases -> https://github.com/getsentry/sentry-docs/tree/master/src/includes/getting-started-install

if the install steps don't include 2 dependencies or more, we don't add the bom, but rather to the integrations or guides that do, eg https://github.com/getsentry/sentry-docs/blob/master/src/platforms/android/configuration/integrations/fragment.mdx
we'd just need to find all the files that add 2 or more dependencies, and update them to use the bom

@maciejwalkowiak

Copy link
Copy Markdown
Contributor Author

I see a scenario where users uses Spring and Logback - there is no dedicated page for this combination, and there wouldn't be a place to put BOM docs.

What about having a separate section "Using BOM" or "Managing Dependencies" somewhere in the main menu:

image

@marandaneto

Copy link
Copy Markdown
Contributor

I see a scenario where users uses Spring and Logback - there is no dedicated page for this combination, and there wouldn't be a place to put BOM docs.

What about having a separate section "Using BOM" or "Managing Dependencies" somewhere in the main menu:

image

that makes sense, not sure how the BOM word is known, but something related to Dependencies makes sense to me.

@maciejwalkowiak

Copy link
Copy Markdown
Contributor Author

@marandaneto updated.

@bruno-garcia bruno-garcia 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.

I think this is something @PeloWriter and @marandaneto might have thoughts but if they are happy with it, it also LGTM

One question/suggestion about Android.
Another detail is to possibly not have it as a top level item. Maybe add under configuration?

Comment thread src/platforms/android/common/dependencies.mdx Outdated
@bruno-garcia
bruno-garcia requested a review from PeloWriter July 13, 2021 21:55
Comment thread src/platforms/android/common/dependencies.mdx Outdated
Comment thread src/platforms/android/common/dependencies.mdx Outdated
Comment thread src/platforms/android/common/dependencies.mdx Outdated
Comment thread src/platforms/android/common/dependencies.mdx Outdated
Comment thread src/platforms/android/common/dependencies.mdx Outdated
Comment thread src/platforms/android/common/dependencies.mdx Outdated
Comment thread src/platforms/java/common/dependencies.mdx Outdated
Co-authored-by: Fiona <61481573+PeloWriter@users.noreply.github.com>
maciejwalkowiak and others added 4 commits July 14, 2021 13:50
Co-authored-by: Fiona <61481573+PeloWriter@users.noreply.github.com>
Co-authored-by: Fiona <61481573+PeloWriter@users.noreply.github.com>
Co-authored-by: Fiona <61481573+PeloWriter@users.noreply.github.com>
@maciejwalkowiak

Copy link
Copy Markdown
Contributor Author

@PeloWriter @marandaneto @bruno-garcia please take another look.

@bruno-garcia bruno-garcia 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.

👍

@bruno-garcia
bruno-garcia merged commit 21bf9c3 into getsentry:master Jul 21, 2021
@bruno-garcia

Copy link
Copy Markdown
Member

@maciejwalkowiak seems we didn't ship bom yet though: https://github.com/getsentry/sentry-java/pull/1486/files

I jumped the gun here by merging this, should I revert?

@maciejwalkowiak

Copy link
Copy Markdown
Contributor Author

@bruno-garcia I believe it should be reverted, and merged only once BOM PR is merged and 5.1.0 is released.

taylangocmen pushed a commit that referenced this pull request Jul 22, 2021
Co-authored-by: Fiona <61481573+PeloWriter@users.noreply.github.com>
bruno-garcia added a commit that referenced this pull request Jul 26, 2021
@bruno-garcia

Copy link
Copy Markdown
Member

Raised #3939

@bruno-garcia

Copy link
Copy Markdown
Member

Draft to add this back once 5.1.0 is out: #3940

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Document sentry-bom for Java and Android

4 participants