[GTK Config] Remove redundant reparse configuration call
AbandonedPublic

Authored by gikari on Feb 2 2020, 7:28 PM.

Details

Reviewers
None
Group Reviewers
Plasma
Summary

Since KConfigWatcher automatically reparses configuration on change, this call is redundant.

Test Plan

Check if config changes are detected by changing decoration buttons order

Diff Detail

Repository
R99 KDE Gtk Configuration Tool
Branch
remove-reparse-config (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21977
Build 21995: arc lint + arc unit
gikari created this revision.Feb 2 2020, 7:28 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 2 2020, 7:28 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gikari requested review of this revision.Feb 2 2020, 7:28 PM

You need to be careful here.

Some of this code is re-evaluated on receipt of other direct DBus signals. Not from kconfigwatcher.

In those cases, you will have to reparse.

gikari added a comment.Feb 2 2020, 7:53 PM

You need to be careful here.

Some of this code is re-evaluated on receipt of other direct DBus signals. Not from kconfigwatcher.

In those cases, you will have to reparse.

OK, so that makes 90% of this diff pointless :) Only the decorations button order change are triggered by KConfigWatcher. Would it make sense to port other methods to be triggered by corresponding KConfigWatchers or would it slow down the whole process of syncing settings significantly?

gikari updated this revision to Diff 74877.Feb 2 2020, 8:11 PM

Rollback the reparsings that are actually needed

gikari retitled this revision from Remove redundant reparse configuration calls to Remove redundant reparse configuration call.Feb 2 2020, 8:12 PM
gikari edited the summary of this revision. (Show Details)
gikari edited the test plan for this revision. (Show Details)

Would it make sense to port other methods to be triggered by corresponding KConfigWatchers

Absolutely.
It means changing some stuff upstream to emit the change notifications, but it's the direction I want to take us.

gikari retitled this revision from Remove redundant reparse configuration call to [GTK Config] Remove redundant reparse configuration call.

Ping! After D27904 this should be working.

gikari abandoned this revision.Mon, Mar 9, 8:43 PM

In favor of D27957