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

[MAINTENANCE] Change log level to INFO #1376

Merged
merged 3 commits into from
Dec 16, 2024

Conversation

markusweigelt
Copy link
Contributor

Using default should not be a warning because it signals a problem and distracts from actual issues.

The following warning will be shown in the log.

Setting "displayIiifDescription" not set, using default value "1". Probably FlexForm for controller "Kitodo\Dlf\Controller\MetadataController" is not read. 

I think it's useful to inform about configuration decisions without signaling a problem. I'll adjust the message accordingly.

Change log level cause "WARNING" is too sensive.
@beatrycze-volk
Copy link
Collaborator

Actually it is a real problem, as FlexForm should be configured and used.

@markusweigelt
Copy link
Contributor Author

Actually it is a real problem, as FlexForm should be configured and used.

I think then there is no need for default values here but an error message that this FlexForm must be configured.

@beatrycze-volk
Copy link
Collaborator

Actually it is a real problem, as FlexForm should be configured and used.

I think then there is no need for default values here but an error message that this FlexForm must be configured.

But then without those values the whole page crashes.

@sebastian-meyer sebastian-meyer self-requested a review December 16, 2024 17:29
@sebastian-meyer
Copy link
Member

I am with @markusweigelt here: Falling back to default values should not be considered an error. If it was we shouldn't provide default values, but immediately throw an exception when mandatory settings are not available.

@sebastian-meyer
Copy link
Member

Making sure that Flexform settings are read at all (if provided) is a different issue which should not be adressed by warning about the usage of default values. Also, I think not using Flexform at all, but instead providing settings in Typoscript is perfectly fine.

So, in essence: The plugins should make sure that mandatory settings are provided (and throw an error otherwise), but they should not need to worry about where those settings come from (Flexform, Typoscript, Extension config).

@sebastian-meyer sebastian-meyer merged commit 18a6ba4 into kitodo:main Dec 16, 2024
8 checks passed
@sebastian-meyer sebastian-meyer added the 🛠 maintenance A task to keep the code up-to-date and manageable. label Dec 16, 2024
@markusweigelt markusweigelt deleted the patch-1 branch January 17, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 maintenance A task to keep the code up-to-date and manageable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants