-
Notifications
You must be signed in to change notification settings - Fork 524
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
[Docs] 7.0 Breaking changes, Release highlights, Migration assistant, ECS changes #1940
Conversation
sounds good to me @bmorelli25! thanks for looking into this. I guess there's a script from beats to export the changes fields? |
changelogs/7.0.asciidoc
Outdated
@@ -17,6 +27,7 @@ https://github.com/elastic/apm-server/compare/v6.7.0\...v7.0.0[View commits] | |||
[float] | |||
==== Breaking Changes | |||
- Move fields in ES to be ECS compliant {pull}1766[1766],{pull}1783[1783],{pull}1813[1813],{pull}1836[1836],{pull}1838[1838],{pull}1844[1844],{pull}1848[1848],{pull}1849[1849],{pull}1863[1863],{pull}1870[1870] | |||
// ** [More information link would go here (linking to APM overview breaking changes)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it shouldn't be the other way round, to give a highlight in the apm guide, and then link to the exhaustive changes list in the dedicated repositories.
Apart from that I like the suggested changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simitt - I might've done a bad job explaining. I think the exhaustive changes should live in the dedicated repos (where they currently live).
The biggest issue for me was determining whether the link you highlight here should link to the APM Overview breaking changes or the APM Server breaking changes. Ultimately I went we the Overview so that users would also be able to easily see all APM breaking changes in one place. I still added a brief note in the Server breaking changes.
I've added some more commits to this PR that should make my vision clearer. Take a look and let me know your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on your proposal and changes.
One minor note, I think the link from the apm-server to the general APM breaking changes is broken, at least I got a 404 when trying to follow it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is one of the limitations we'll run into when making doc changes in two different books (server & overview) at the same time. Since we can only build one book locally at a time, the new pages in the APM overview will not be built, and the links will 404. I've gone through and manually looked at all the URLs to make sure they look correct, and they do, so I'm hopeful there will be no broken links when this PR is merged 🤞
Good call @roncohen - Ruflin created a |
f2fcf4f
to
bde8dfa
Compare
Removed WIP tag and rebased - how do we feel about merging this soon 👍/ 👎? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please run make fmt check
to ensure the python script fulfills the style guides.
This PR accomplishes the following:
renamed_fields.py
to generate a changed fields tableHere's a general idea of what I'm thinking for release notes and breaking changes moving forward.
There was way too much happening on the old Overview RN page. I moved each release to its own page.
I think it makes sense to add a separate breaking changes section to the APM Overview like we have in the APM Server, and other Elastic stack repos.
A concern I have (and this isn't anything new), is that we have similar information available in many different places.
To "fix" this, I think the APM Overview's Release notes section should be renamed to Release highlights. I understand this may seem like a subtle change (and it is), but it's an important distinction to make. We're currently treating this section as a hybrid Notes/Highlights section, which has led to some duplication of content. On the bright side, this is an easy change to make. I can focus on expanding and explaining changes here, then link in/out to the pertinent release notes. Similarly, I will link release notes to this section for more information.
I like that beats is listing all of the ECS field changes in their 7.0 breaking changes (here) and think that we should do the same. I've got a template up on the breaking changes page, just need to fill in the details.
Added in placeholders for the next few releases to make the process slightly easier.
Thoughts?
cc @roncohen