Add a logging category for logs warnings
ClosedPublic

Authored by meven on Jun 23 2019, 10:28 PM.

Details

Summary

Disable logs unless the logging category kf5.kconfig.core is enabled

Test Plan

ctest

Diff Detail

Repository
R237 KConfig
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13247
Build 13265: arc lint + arc unit
meven created this revision.Jun 23 2019, 10:28 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJun 23 2019, 10:28 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
meven requested review of this revision.Jun 23 2019, 10:28 PM
apol added a subscriber: apol.Jun 23 2019, 10:35 PM
apol added inline comments.
src/core/kdesktopfile.h
27 ↗(On Diff #60545)

Use ecm_qt_declare_logging_category from cmake.

This should never be in header files too, or it would spill on applications otherwise.

See for example how it's done in this same repo on KCONF_UPDATE_LOG.

meven updated this revision to Diff 60546.EditedJun 23 2019, 11:07 PM

Use ecm_qt_declare_logging_category to setup logging

meven updated this revision to Diff 60547.Jun 23 2019, 11:08 PM

Removed blank line

Thank you very much again @apol for your guidance.

I am not sure my naming scheme is ideal.

apol added inline comments.Jun 24 2019, 9:01 AM
src/core/CMakeLists.txt
19

This is for KF5ConfigCore, I'd call it kf5.kconfig.core.

meven updated this revision to Diff 60558.Jun 24 2019, 9:09 AM

Use KCONF_CORE_LOG as identifier and kf5.kconfig.core as category_name

meven edited the summary of this revision. (Show Details)Jun 24 2019, 9:10 AM
meven marked an inline comment as done.
apol added a comment.Jun 24 2019, 9:20 AM

The patch looks good to me.

I see that you're just doing KDesktopFile, but there's many others available, can you port the rest too?

meven updated this revision to Diff 60610.Jun 24 2019, 6:10 PM

Add logging category for all kconfig core warnings message

This would be nice to have.

meven edited the test plan for this revision. (Show Details)Jul 17 2019, 10:00 AM
meven updated this revision to Diff 61897.Jul 17 2019, 10:08 AM

Add a logging categorie, add context to logs

meven added a reviewer: apol.Jul 30 2019, 7:04 AM
krop added a subscriber: krop.Jul 30 2019, 7:23 AM
krop added inline comments.
CMakeLists.txt
102 ↗(On Diff #61897)

This line is not useful

src/core/CMakeLists.txt
17

the framework is called kconfig, the public headers are called kconfig*.h, there's no reason to truncate the framework name.

Also, why 'deskop' in the name ? this is for kconfigcore. kconfigcore_debug.h sounds better

meven updated this revision to Diff 62903.Aug 1 2019, 3:14 PM

Rename kconf_desktop_debug.h to more appropriate kconfig_core_log_settings.h, do not run an unnecessary install

meven marked 2 inline comments as done.Aug 1 2019, 3:15 PM

I have opted for the file name kconfig_core_log_settings.h for the log settings header file.

krop added a comment.Aug 14 2019, 1:54 PM

ok, please change the remaining kconf/KCONF to kconfig/KCONFIG and it should be fine.

meven updated this revision to Diff 63729.Aug 14 2019, 2:43 PM

Renamed kconf to kconfig, KCONF to KCONFIG, re-add the mistakenly removed categories file

meven added a comment.Aug 14 2019, 2:45 PM
In D22061#511799, @cgiboudeaux wrote:

ok, please change the remaining kconf/KCONF to kconfig/KCONFIG and it should be fine.

Should be good now.
Thank you for your feedback.

krop accepted this revision.Aug 14 2019, 3:12 PM

LGTM

This revision is now accepted and ready to land.Aug 14 2019, 3:12 PM
This revision was automatically updated to reflect the committed changes.