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.

Sign up for GitHub

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

Jump to bottom

[7.x] Ensure deprecation documentationUrl's point to correct doc branch #114263

Merged
merged 4 commits into from
Oct 12, 2021
Merged

[7.x] Ensure deprecation documentationUrl's point to correct doc branch #114263

merged 4 commits into from
Oct 12, 2021

Conversation

watson
Copy link
Contributor

@watson watson commented Oct 7, 2021 •
edited
Loading

This was made possible via PR #113600.

This PR does not do anything about the documenationUrl usage in the following files as they do not currently have access to ConfigDeprecationContext:

This PR goes directly into the 7.x branch. In regards to deprecation messages there's too many differences between master and 7.x to warrant a single PR to master that's then backported to 7.x. I've made another PR specifically for master: #114264.

@watson watson added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v7.16.0 labels Oct 7, 2021
@watson watson self-assigned this Oct 7, 2021
@watson watson mentioned this pull request Oct 7, 2021
Merged
@watson watson requested review from a team October 7, 2021 11:51
@watson watson mentioned this pull request Oct 7, 2021
Merged
2 tasks
azasypkin
azasypkin approved these changes Oct 7, 2021
View reviewed changes
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Security plugin code changes LGTM

@watson
Copy link
Contributor Author

watson commented Oct 7, 2021

I got some I18n test errors because - as far as I can see - I've changed the input to i18n.translate() to include expressions inside of template literals.

I get why this can be problematic because these strings are to be translated. But in any case, I guess the right approach is to remove the URLs from these all together since they seem to just be duplicates of the URL in documentationUrl.

I've fixed this in this new commit to the PR: d21c627.

@watson
Copy link
Contributor Author

watson commented Oct 7, 2021 •
edited
Loading

FYI: @elastic/apm-ui + @elastic/kibana-reporting-services + @elastic/datavis I've not been able to update your deprecation messages in this PR because they do not use the same setup with access to ConfigDeprecationContext as the newer deprecation messages (see PR description for file links). You probably want to look into that before 7.16 FF.

There's a similar PR for master, which is obviously less critical: #114264.

nreese
nreese approved these changes Oct 7, 2021
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

maps changes LGTM
code review

pgayvallet
pgayvallet approved these changes Oct 7, 2021
View reviewed changes
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

Thanks for doing this.

@watson watson enabled auto-merge (squash) October 7, 2021 15:47
@watson watson disabled auto-merge October 7, 2021 15:49
@watson watson enabled auto-merge (squash) October 7, 2021 16:53
@pgayvallet
Copy link
Contributor

@elasticmachine merge upstream

@pgayvallet
Copy link
Contributor

retest

@watson
Copy link
Contributor Author

watson commented Oct 12, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @watson

@watson watson merged commit f64c66c into elastic:7.x Oct 12, 2021
@watson watson deleted the deprecation-url-branch-7x branch October 12, 2021 09:33
@sorenlouv sorenlouv mentioned this pull request Oct 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azasypkin azasypkin azasypkin approved these changes

@pgayvallet pgayvallet pgayvallet approved these changes

@nreese nreese nreese approved these changes

Assignees

@watson watson

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v7.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@watson @pgayvallet @kibanamachine @nreese @azasypkin