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

[App Search] Version documentation links #83245

Merged
merged 3 commits into from
Nov 12, 2020

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Nov 12, 2020

Summary

Adds a DRY version prefix to our outgoing App Search documentation URLs. I believe @chriscressman noted that we should be versioning our doc URLs going forward (instead of using current) so that older users land up on the correct docs and don't get dead links.

Workplace Search is currently already doing this, so we're just catching up/copying them 🐒

QA

- Was previously just sending (e.g.) "7". instead of "7.9"
- except for Enterprise Search setup guide, which should be updated to use a shared URL at some point in any case
@cee-chen cee-chen requested a review from a team November 12, 2020 05:02
@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 12, 2020
@@ -8,4 +8,4 @@ import { SemVer } from 'semver';
import pkg from '../../../../package.json';

export const CURRENT_VERSION = new SemVer(pkg.version as string);
export const CURRENT_MAJOR_VERSION = CURRENT_VERSION.major;
export const CURRENT_MAJOR_VERSION = `${CURRENT_VERSION.major}.${CURRENT_VERSION.minor}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@scottybollinger FYI - links using this const for URLs were previously faulty. They were generating links that looked like

https://www.elastic.co/guide/en/app-search/7/
instead of
https://www.elastic.co/guide/en/app-search/7.9/

I should have fixed it here but let me know if you see any issues.

Also the var name is technically not correct now so IDK if we should change it lol. Is CURRENT_MAJOR_MINOR_VERSION too verbose? Maybe we should do CURRENT_FULL_VERSION and CURRENT_VERSION?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will just leave this var name for now but let me know if you want to change this and I'll open a new PR

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 695.7KB 696.3KB +566.0B

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

Copy link
Member

@JasonStoltz JasonStoltz left a comment

Choose a reason for hiding this comment

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

Good call.

@cee-chen cee-chen merged commit bd99b19 into elastic:master Nov 12, 2020
@cee-chen cee-chen deleted the semver-as-docs branch November 12, 2020 15:42
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 12, 2020
* master:
  [Ingest Manager] Lift up registry/{stream,extract} functions (elastic#83239)
  [Reporting] Move "common" types and constants to allow cross-plugin integration (elastic#83198)
  [Lens] Add suffix formatter (elastic#82852)
  [App Search] Version documentation links (elastic#83245)
  Use saved object references for dashboard drilldowns (elastic#82602)
  Btsymbala/registered av (elastic#81910)
  [APM] Errors table for service overview (elastic#83065)
cee-chen pushed a commit that referenced this pull request Nov 12, 2020
* Fix CURRENT_MAJOR_VERSION for use in Elastic docs links

- Was previously just sending (e.g.) "7". instead of "7.9"

* Add App Search DOCS_PREFIX constant

- follow WS's example

* Update all App Search doc links to use prefixed URLs

- except for Enterprise Search setup guide, which should be updated to use a shared URL at some point in any case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants