Skip to content

Conversation

@karenzone
Copy link
Contributor

@karenzone karenzone commented Sep 9, 2019

Related to #1169
This PR replaces the hard-coded ECS version number with an attribute that supports versioning.

DO NOT MERGE until all books using the hard-coded ECS attribute have been updated to include //shared/versions/stack/{source_branch}.asciidoc (or handled in product repo versioning)

@karenzone karenzone requested a review from lcawl September 9, 2019 19:52
@karenzone
Copy link
Contributor Author

@webmat FYI: More steps toward implementing ECS versioning

@karenzone
Copy link
Contributor Author

karenzone commented Sep 9, 2019

@dedemorton Here are the ECS versioning changes. Both LS and Beats handle versioning in the repo. Toward your vision outlined in #804, I'm considering switching Logstash to use this versioning approach. I'd appreciate your eyes on this PR (elastic/logstash#11125) to make sure I'm not overlooking any constraints.

@karenzone
Copy link
Contributor Author

run elasticsearch-ci/docs rebuild

@karenzone
Copy link
Contributor Author

karenzone commented Sep 17, 2019

@bmorelli25 If I'm interpreting the CI errors correctly, we might need to add a link to shared versions (include::{asciidoc-dir}/../../shared/versions/stack/{source_branch}.asciidoc[]) in https://raw.githubusercontent.com/elastic/apm-agent-python/master/docs/index.asciidoc

If that's the right fix, I'm happy to make the fix and tag you as a reviewer. Let's discuss.

@bmorelli25
Copy link
Member

Ahhh good find. Yup, looks like we probably need to include that file in all of the APM Agents to be safe.

@bmorelli25
Copy link
Member

bmorelli25 commented Sep 18, 2019

Will that work though? There are no asciidoc files in https://github.com/elastic/docs/tree/master/shared/versions/stack that match the current python version of 5.x. Same for the other APM Agent versions. They're all <5.

This why we currently hardcode :branch: current in https://raw.githubusercontent.com/elastic/apm-agent-python/master/docs/index.asciidoc.

@bmorelli25
Copy link
Member

bmorelli25 commented Sep 18, 2019

@karenzone Now that you've pointed this out, I'm seeing the same problem in #1187 with Agent to Agent links also failing because they don't include the shared version file.

I noticed you have the ECS docs including 7.x version file. I'm not sure that makes sense for APM. I see there was a lot of discussion around it in elastic/ecs#550.

I almost wonder if it makes sense to add another file to https://github.com/elastic/docs/tree/master/shared/versions/stack called other_versions (or something like that). This file could then "include" the correct current version file that folder? I'm not sure if that makes sense, but I'm trying to figure out how we can have all versions be set from the docs repo.

@karenzone
Copy link
Contributor Author

run elasticsearch-ci/docs rebuild

@bmorelli25
Copy link
Member

@karenzone I just merged the backport. You may want to restart this build. I think it will fail as it was started before that PR was merged.

@karenzone
Copy link
Contributor Author

run elasticsearch-ci/docs rebuild

@karenzone karenzone changed the title Replace ecs version with attribute [DO NOT MERGE] Replace ecs version with attribute Sep 18, 2019
Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

LGTM, congrats!

@karenzone karenzone merged commit 22e670a into elastic:master Sep 18, 2019
@karenzone karenzone deleted the ecs-attribute branch September 18, 2019 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants