Differential D3357

Don't register a new service for screenshots
ClosedPublic
Actions

Authored by davidedmundson on Nov 14 2016, 10:29 AM.

Details

Reviewers
graesslin
Group Reviewers
Plasma
Commits
R108:aae24bb07d60: Don't register a new service for screenshots
Summary

If you're going to have kwin in the service name anyway there's
absolutely no benefit to having clients not use the existing
org.kde.KWin.

Test Plan

Object is still registered.

Diff Detail

Repository
R108 KWin
Branch
scaling_2
Lint
No Linters Available
Unit
No Unit Test Coverage
davidedmundson updated this revision to Diff 8133.Nov 14 2016, 10:29 AM
davidedmundson retitled this revision from to Don't register a new service for screenshots.
davidedmundson updated this object.
davidedmundson edited the test plan for this revision. (Show Details)
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: KWin. · View Herald TranscriptNov 14 2016, 10:29 AM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
graesslin added a subscriber: graesslin.Nov 14 2016, 5:01 PM

While not perfect I do consider this as a dangerous change. It's kind of our public API. According to lxr.kde.org it's not used in our code, so from that side not a problem.

My suggestion would be to hold the change a little bit. We need to change the DBus API for Wayland anyway. And as people are loudly complaining about the lack of screenshot I will try to implement an API shortly. Let's combine it then and make it a dedicated DBus API break.

graesslin accepted this revision.Nov 15 2016, 6:17 AM
graesslin added a reviewer: graesslin.

On second thought: we can push it directly

This revision is now accepted and ready to land.Nov 15 2016, 6:17 AM
Closed by commit R108:aae24bb07d60: Don't register a new service for screenshots (authored by davidedmundson). · Explain WhyNov 25 2016, 10:15 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Meffects/screenshot/screenshot.cpp (2 lines)

Diff 8133

View Options

effects/screenshot/screenshot.cpp

Loading...