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

Fix for rotate_age where Fluentd passes as Symbol #4311

Merged
merged 5 commits into from
Oct 18, 2023

Conversation

mrudrego
Copy link
Contributor

@mrudrego mrudrego commented Sep 26, 2023

Fix for rotate_age where Fluentd passes as Symbol while Ruby Logger expects String

Which issue(s) this PR fixes:
Fixes #4227

What this PR does / why we need it:

Docs Changes:

Release Note:

@daipom daipom self-requested a review September 27, 2023 09:25
@daipom
Copy link
Contributor

daipom commented Sep 27, 2023

Thanks!
Could you add DCO to all commits?
Or you can rebase the commits to one commit.

@mrudrego
Copy link
Contributor Author

@daipom , have added the DCO to the commits. The second commit do not have any changes actually, since i had missed to add DCO on the first commit, tried to add in the next assuming it would take for other also.

Thanks,

@mrudrego
Copy link
Contributor Author

@daipom, have pushed code changes for the tests which were failing and on my local windows env, these loglevel tests have passed. Please do the needful.
Thanks,

@daipom
Copy link
Contributor

daipom commented Oct 4, 2023

@mrudrego Thanks for fixing the tests, but the new commit does not have DCO.
Could you add DCO?

@daipom daipom added the bug Something isn't working label Oct 4, 2023
mrudrego and others added 3 commits October 5, 2023 11:01
@mrudrego
Copy link
Contributor Author

mrudrego commented Oct 9, 2023

@daipom can you please provide approval for the further checks.

Thanks,

@mrudrego
Copy link
Contributor Author

Hi @daipom, can you please do the needful.

Thanks,

lib/fluent/system_config.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

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

Sorry for the delay.
I think we don't need to_s.
In these contexts, the value should be originally String.

lib/fluent/system_config.rb Outdated Show resolved Hide resolved
test/config/test_system_config.rb Outdated Show resolved Hide resolved
@daipom daipom added this to the v1.16.3 milestone Oct 17, 2023
@daipom daipom self-requested a review October 18, 2023 06:20
Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks so much for this fix!

@daipom daipom merged commit dd1a6e5 into fluent:master Oct 18, 2023
13 of 16 checks passed
daipom pushed a commit to daipom/fluentd that referenced this pull request Oct 31, 2023
Fix for rotate_age where Fluentd passes as Symbol while Ruby Logger expects String

---------

Signed-off-by: mrudrego <[email protected]>
Signed-off-by: mrudrego <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot use "enum" for specifying "rotate_age".
3 participants