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

Add support for Journald input #302

Merged
merged 7 commits into from
Sep 20, 2022
Merged

Conversation

mindriot88
Copy link
Contributor

The initial purpose of this change was to add support for the journald input which was introduced in 7.16, however I personally don't think it makes sense to split out the input template for version 6 config and filestream config when the configuration syntax is so nearly identical, the separate template for version 5 is reasonable.

I have also taken the liberty of correcting some of versioning constraints put in around the usage of the filestream input, as it was actually added in 7.10 but this module restricted its use to version 7.16+, this has been mentioned in #299

I have also added support for global configuration keep_null and index as described in #287

I will do my best to address any feedback in a timely manor.

@pcfens
Copy link
Owner

pcfens commented Sep 13, 2022

This looks awesome and is a huge help.

Could you do 2 things that will help with the merge? The first is just updating the README to reflect the new input parameters so the docs stay up to date. To make merging easier can you rebase as well? I suspect that's from your fork pre-dating some other recent change made in this copy.

Thanks again!

@mindriot88
Copy link
Contributor Author

This looks awesome and is a huge help.

Could you do 2 things that will help with the merge? The first is just updating the README to reflect the new input parameters so the docs stay up to date. To make merging easier can you rebase as well? I suspect that's from your fork pre-dating some other recent change made in this copy.

Thanks again!

No problem, ill do that.

In terms of my approach to resolving the conflicts, this what I suggest:

Does this sound reasonable?

If I have time I can look at throwing in a test to validate the correct template is used for the various versions supported.

@mindriot88
Copy link
Contributor Author

@pcfens Apologies for delay, the merge conflicts are resolved.

@pcfens pcfens merged commit 3db9d9a into pcfens:master Sep 20, 2022
@JGodin-C2C JGodin-C2C mentioned this pull request Nov 9, 2022
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