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

fixes #29938 - change default logging layout #847

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

domitea
Copy link
Contributor

@domitea domitea commented May 27, 2020

Complementary PR for theforeman/foreman#7220 to make default new kind of logger mentioned in liked PR

@ekohl
Copy link
Member

ekohl commented May 27, 2020

If we want to make this active for all existing installations, it also needs an installer migration.

@tbrisker
Copy link
Member

Core has been merged, @domitea please add the migration as suggested

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Since this layout added only in 2.2, please add compatibility notes to the README.

@domitea
Copy link
Contributor Author

domitea commented Aug 5, 2020

Migration added in theforeman/foreman-installer#557, also Readme.md was affected by this change.

README.md Outdated
@@ -74,7 +74,7 @@ previous stable release.

### Foreman version compatibility notes

This module targets Foreman 2.0+.
This module targets Foreman 2.2+.
Copy link
Member

Choose a reason for hiding this comment

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

We don't do this, see the support policy. Instead, we would write a new paragraph that on Foreman versions older than 2.2 you need to change the logging layout parameter to pattern.

@ekohl
Copy link
Member

ekohl commented Aug 5, 2020

And you need to add the new value to the valid values in Enum.

@ekohl ekohl mentioned this pull request Aug 5, 2020
@domitea
Copy link
Contributor Author

domitea commented Aug 6, 2020

@ekohl thanks for mentioning enum! Readme updated.

@tbrisker
Copy link
Member

tbrisker commented Sep 7, 2020

@ekohl what's the status here?

@ekohl ekohl merged commit db41ee8 into theforeman:master Oct 21, 2020
@ekohl
Copy link
Member

ekohl commented Oct 21, 2020

It sort of slipped form my radar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants