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

Remove duplicate definitions of os from host.os and user_agent.os... #168

Merged
merged 3 commits into from
Nov 6, 2018

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Nov 5, 2018

With os now a reuseable field set, it's now expected in these locations. The field set should only be defined once.

Having it (and other reuseable field sets) appear correctly in template.json is still work to be done.

@webmat webmat mentioned this pull request Nov 5, 2018
26 tasks
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM but I would not list the os removal under breaking changes as the fields are still there.

@ruflin
Copy link
Member

ruflin commented Nov 6, 2018

As discussed on Slack, I would suggest to list it under Improvements in the changelog.

@webmat
Copy link
Contributor Author

webmat commented Nov 6, 2018

@ruflin You were asking if the stuff removed was actually in line with the os field set. Well both of these dupe definitions were actually missing one field already :-)

Mathieu Martin added 3 commits November 6, 2018 09:07
…s`...

With `os` now a reuseable object, it's now expected in these locations, and the field set should only be declared once.

Having it (and other reuseable objects) appear correctly in `template.json` is still work to be done.
It's not an actual breaking change, it's true.
@webmat webmat merged commit 08c58e4 into elastic:master Nov 6, 2018
@webmat webmat deleted the dupe-reuseable-os branch November 6, 2018 14:52
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.

2 participants