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

Clarify documentation for the observability:enableComparisonByDefault setting #161372

Merged
merged 5 commits into from
Jul 7, 2023

Conversation

felixbarny
Copy link
Member

@felixbarny felixbarny commented Jul 6, 2023

The previous description

Enables the comparison feature in the APM app.

makes it sound like the entire feature would be disabled when setting the option to false.

…t` setting

The previous description
> Enables the comparison feature in the APM app.

makes it sound like the entire feature would be disabled when setting the option to false.
@felixbarny felixbarny added Team:APM All issues that need APM UI Team support docs labels Jul 6, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2023

Documentation preview:

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@bmorelli25
Copy link
Member

Mind if I push an update to this PR? The settings definitions in the docs should match what we show in the UI. To do that, we'll need to update this line as well:

defaultMessage: 'Enable the comparison feature in APM app',

@bmorelli25 bmorelli25 added release_note:skip Skip the PR/issue when compiling release notes v8.9.0 v8.8.3 labels Jul 6, 2023
@felixbarny
Copy link
Member Author

Sure, feel free to push any updates directly to the PR. Thanks!

@bmorelli25 bmorelli25 requested a review from a team as a code owner July 6, 2023 22:01
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 408 412 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 487 491 +4
total +6

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@felixbarny felixbarny enabled auto-merge (squash) July 7, 2023 07:11
Copy link
Member

@maryam-saeidi maryam-saeidi left a comment

Choose a reason for hiding this comment

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

LGTM, just added a clarifying question, please check that before merging :)

Haha, missed that it is auto-merged!

@@ -76,7 +76,8 @@ export const uiSettings: Record<string, UiSettings> = {
}),
value: true,
description: i18n.translate('xpack.observability.enableComparisonByDefaultDescription', {
defaultMessage: 'Enable the comparison feature in APM app',
defaultMessage:
Copy link
Member

Choose a reason for hiding this comment

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

Since the key has not been changed, the translation might not be updated in main (discussion)

Also, for the previous releases, since the translation is already done, there are basically two options:

  • Removing the translation and, as a result, showing the English version by default
  • Keep the current translation if it is OK to have different messages in different languages

Is the second option what you want?

@felixbarny felixbarny merged commit 5c0f034 into main Jul 7, 2023
@felixbarny felixbarny deleted the clarify-docs-for-comparison-setting branch July 7, 2023 07:52
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 7, 2023
…t` setting (elastic#161372)

The previous description
> Enables the comparison feature in the APM app.

makes it sound like the entire feature would be disabled when setting
the option to false.

(cherry picked from commit 5c0f034)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
8.8 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.8:
- [APM] Documentation updates (#160951)
8.9

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 161372

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jul 7, 2023
…Default` setting (#161372) (#161433)

# Backport

This will backport the following commits from `main` to `8.9`:
- [Clarify documentation for the
`observability:enableComparisonByDefault` setting
(#161372)](#161372)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Felix
Barnsteiner","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-07-07T07:52:47Z","message":"Clarify
documentation for the `observability:enableComparisonByDefault` setting
(#161372)\n\nThe previous description\r\n> Enables the comparison
feature in the APM app.\r\n\r\nmakes it sound like the entire feature
would be disabled when setting\r\nthe option to
false.","sha":"5c0f034e7517898f25f664ebc9d7c31f5b3d33ad","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:APM","release_note:skip","docs","v8.9.0","v8.10.0","v8.8.3"],"number":161372,"url":"https://github.com/elastic/kibana/pull/161372","mergeCommit":{"message":"Clarify
documentation for the `observability:enableComparisonByDefault` setting
(#161372)\n\nThe previous description\r\n> Enables the comparison
feature in the APM app.\r\n\r\nmakes it sound like the entire feature
would be disabled when setting\r\nthe option to
false.","sha":"5c0f034e7517898f25f664ebc9d7c31f5b3d33ad"}},"sourceBranch":"main","suggestedTargetBranches":["8.9","8.8"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/161372","number":161372,"mergeCommit":{"message":"Clarify
documentation for the `observability:enableComparisonByDefault` setting
(#161372)\n\nThe previous description\r\n> Enables the comparison
feature in the APM app.\r\n\r\nmakes it sound like the entire feature
would be disabled when setting\r\nthe option to
false.","sha":"5c0f034e7517898f25f664ebc9d7c31f5b3d33ad"}},{"branch":"8.8","label":"v8.8.3","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Felix Barnsteiner <[email protected]>
@gbamparop
Copy link
Contributor

Tested on edge-lite:

image

@gbamparop gbamparop added the apm:test-plan-done Pull request that was successfully tested during the test plan label Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:test-plan-done Pull request that was successfully tested during the test plan docs release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v8.8.3 v8.9.0 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants