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

Switch to YAML config files #1059

Merged
merged 18 commits into from
Sep 21, 2020
Merged

Switch to YAML config files #1059

merged 18 commits into from
Sep 21, 2020

Conversation

mavam
Copy link
Member

@mavam mavam commented Sep 15, 2020 •
edited
Loading

@mavam mavam added blocked Blocked by an (external) issue enhancement ✨ feature New functionality and removed enhancement ✨ labels Sep 15, 2020
@mavam mavam force-pushed the story/ch19445 branch 2 times, most recently from 3c0b82c to 077341d Compare September 16, 2020 06:36
@mavam mavam removed the blocked Blocked by an (external) issue label Sep 16, 2020
@mavam mavam force-pushed the story/ch19445 branch 2 times, most recently from 7671b78 to 25326c2 Compare September 18, 2020 10:29
@mavam
Copy link
Member Author

mavam commented Sep 18, 2020

Note to the reviewer: this PR is best reviewed commit by commit.

dominiklohmann
dominiklohmann requested changes Sep 18, 2020
View reviewed changes
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

I only looked at the code for now, will do the testing locally for a later review round.

Please rename the config file in this PR already and warn if a vast.conf file is found to reduce friction for the user.

vast.conf.example Outdated Show resolved Hide resolved
vast.conf.example Outdated Show resolved Hide resolved
vast.conf.example Outdated Show resolved Hide resolved
libvast/src/system/configuration.cpp Outdated Show resolved Hide resolved
libvast/src/system/configuration.cpp Outdated Show resolved Hide resolved
libvast/src/system/configuration.cpp Outdated Show resolved Hide resolved
@mavam mavam added the blocked Blocked by an (external) issue label Sep 18, 2020
@mavam
Copy link
Member Author

mavam commented Sep 18, 2020

CI currently fails due the issue to be fixed in #1067; marking this PR as blocked until merged.

@mavam mavam removed the blocked Blocked by an (external) issue label Sep 19, 2020
@mavam mavam marked this pull request as ready for review September 19, 2020 14:10
dominiklohmann
dominiklohmann approved these changes Sep 21, 2020
View reviewed changes
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

This is working nicely in practice. I've manually verified the merging logic to work.

@mavam mavam merged commit aed7448 into master Sep 21, 2020
@mavam mavam deleted the story/ch19445 branch September 21, 2020 09:43
@mavam mavam mentioned this pull request Sep 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominiklohmann dominiklohmann dominiklohmann approved these changes

Assignees
No one assigned
Labels
feature New functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mavam @dominiklohmann