D8051: Custom background color

2017-10-01 Thread Luigi Toscano
ltoscano added a comment. It is, but when you did `arc patch `, the patch committed locally ended up with you as author because the patch had no authorship information. So yes, arc patch, then check the author and if incorrect fix it (see the message in

D8051: Custom background color

2017-10-01 Thread Nathaniel Graham
ngraham added a comment. Sorry about that, guys. I just did `arc land`. Was that not the right approach to land this? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular, aacid, elvisangelaccio, rkflx, ngraham Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-10-01 Thread Luigi Toscano
ltoscano added a comment. Both are In https://phabricator.kde.org/D8051#151095, @rkflx wrote: > In https://phabricator.kde.org/D8051#151091, @ltoscano wrote: > > > this was committed with the wrong authorship. please revert and commit again with the right author. > > > I

D8051: Custom background color

2017-10-01 Thread Henrik Fehlauer
rkflx added a comment. In https://phabricator.kde.org/D8051#151091, @ltoscano wrote: > this was committed with the wrong authorship. please revert and commit again with the right author. I already raised this in

D8051: Custom background color

2017-10-01 Thread Albert Freeman
albertfreeman added a comment. I didn't use Arcanist. I recreated it via the phabricator.kde.org interface. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular, aacid, elvisangelaccio, rkflx, ngraham Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-10-01 Thread Luigi Toscano
ltoscano added a comment. Ok, reverted and repushed. @albertfreeman I've got your email from the old review on phabricator. Unfortunately this review did not contain the details. How did you send this review here? Did you use arcanist? REPOSITORY R223 Okular REVISION DETAIL

D8051: Custom background color

2017-10-01 Thread Luigi Toscano
ltoscano added a comment. this was committed with the wrong authorship. please revert and commit again with the right author. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular, aacid, elvisangelaccio, rkflx, ngraham Cc: aacid, ltoscano,

D8051: Custom background color

2017-09-30 Thread Nathaniel Graham
This revision was automatically updated to reflect the committed changes. Closed by commit R223:6b5a7c9a1a00: Custom background color (authored by ngraham). REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8051?vs=20174=20175 REVISION DETAIL

D8051: Custom background color

2017-09-30 Thread Nathaniel Graham
ngraham accepted this revision. This revision is now accepted and ready to land. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular, aacid, elvisangelaccio, rkflx, ngraham Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-09-30 Thread Albert Freeman
albertfreeman updated this revision to Diff 20174. albertfreeman added a comment. Rename slot to setCustomBackgroundColorButton. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8051?vs=20143=20174 REVISION DETAIL https://phabricator.kde.org/D8051

D8051: Custom background color

2017-09-30 Thread Nathaniel Graham
ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. @aacid thanks for the info. Master it is, then. @albertfreeman, can you rename useCustomBackgroundColor() to something different? I agree with Albert that the "set" prefix

D8051: Custom background color

2017-09-30 Thread Albert Astals Cid
aacid added a comment. In https://phabricator.kde.org/D8051#150925, @ngraham wrote: > The patch applies cleanly and the feature works well. Nice job! > > @aacid and @rkflx, if there are no other objections, should we get this into 17.08, or just master? This is not a bugfix,

D8051: Custom background color

2017-09-30 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. The patch applies cleanly and the feature works well. Nice job! @aacid and @rkflx, if there are no other objections, should we get this into 17.08, or just master? REPOSITORY R223

D8051: Custom background color

2017-09-30 Thread Albert Freeman
albertfreeman edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular, aacid, elvisangelaccio, rkflx Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-09-30 Thread Albert Freeman
albertfreeman updated this revision to Diff 20143. albertfreeman added a comment. Removed blank line from conf/okular.kcfg. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8051?vs=20077=20143 REVISION DETAIL https://phabricator.kde.org/D8051 AFFECTED

D8051: Custom background color

2017-09-29 Thread Henrik Fehlauer
rkflx added a comment. Thanks for bringing this to Phabricator and implementing my suggestion on such short notice. I tested your patch and cannot find anything wrong with it in terms of behaviour. For the code if have a trivial nitpick (see below) and I have a comment regarding the commit

D8051: Custom background color

2017-09-29 Thread Nathaniel Graham
ngraham added reviewers: aacid, elvisangelaccio, rkflx. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular, aacid, elvisangelaccio, rkflx Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-09-29 Thread Albert Freeman
albertfreeman updated this revision to Diff 20077. albertfreeman added a comment. Reverted changes of accelerators that my Qt Designer automatically made. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8051?vs=20070=20077 REVISION DETAIL

D8051: Custom background color

2017-09-29 Thread Albert Freeman
albertfreeman edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-09-29 Thread Albert Freeman
albertfreeman edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-09-29 Thread Albert Freeman
albertfreeman edited the summary of this revision. albertfreeman added a subscriber: aacid. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular Cc: aacid, ltoscano, ngraham

D8051: Custom background color

2017-09-29 Thread Albert Freeman
albertfreeman retitled this revision from "Custom background color to be enabled and changed from settings" to "Custom background color". REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular Cc: ltoscano, ngraham, aacid

D8051: Custom background color to be enabled and changed from settings

2017-09-29 Thread Albert Freeman
albertfreeman retitled this revision from "Enable custom background color to be enabled and changed from settings" to "Custom background color to be enabled and changed from settings". REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D8051 To: albertfreeman, #okular Cc: