Re: KOpeningHours in kdereview

2020-12-10 Thread Rolf Eike Beer
> For the flex/bison code it might be possible to factor out these parts, as
> it's largely very basic C++ code anyway. You'd need to rebuild most of the
> logic around that though I'm afraid.

Everything that works and can help me spot syntax errors help. If I can't 
parse some things then it is not worse than now where it it just an opaque 
string.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: KOpeningHours in kdereview

2020-12-10 Thread Rolf Eike Beer
> but meanwhile it's also being
> evaluated for use in OSM validation tooling:
> https://github.com/osm-fr/osmose-backend/issues/555. That has already
> resulted in a number of contributions increasing the tolerance for
> non-conforming expressions, which benefits all other use-cases as well.

I was thinking about doing something similar for OSM2go, which currently also 
runs on the N900 and needs gcc 4.2 for it. Is there interest to have a C++98 
STL only version of the main parser? Or is it so tied to Qt classes that there 
is no chance in that?

Eike

signature.asc
Description: This is a digitally signed message part.


Re: KOSMIndoorMap in kdereview

2020-10-22 Thread Rolf Eike Beer
Am Donnerstag, 22. Oktober 2020, 17:25:32 CEST schrieb Volker Krause:
> Hi,
> 
> KOSMIndoorMap is a QML component for showing multi-floor OSM indoor maps (as
> its very creative name might suggest). It's using maps.kde.org as a data
> source (same as Marble), and has been created to show interactive maps of
> train stations for KDE Itinerary.
> 
> https://invent.kde.org/libraries/kosmindoormap
> 
> KOSMIndoorMap has been growing inside the KPublicTransport repo (due to some
> building blocks being available there alreay and me being lazy), and has
> now been split into its own repo. So technically this is coming out of a
> reviewed repo already rather than from playground, but better be on the
> safe side :)
> 
> The aim is having this (together with KPublicTransport and KDE Itinerary)
> join the release service for 20.12.

The const version of DataSet::way() returns Way*, but the non-const version 
returns OSM::Way*. I guess the extra qualification is not needed.

In OSM2go I use std::unordered_map to store the different types of objects 
which makes lookups much easier. YMMV depending if you want to optimize for 
lookup or memory size.

My recent work there is to get rid of the note, way, and relation prefixes or 
suffixes on function names and make a template from the remaining 
implementation so I don't have to implement the same things 3 times.

I have derived all 3 object types from a common base class, which allows me to 
simplify things like Element::url() by just calling obj->url() and let a 
virtual overload do the rest. Which is a good example: the final return in 
that function should be replaced by Q_UNREACHABLE(), too.

The nodes vectors in Element::outerPath() could benefit from a call to 
reserve().

The coding style for the remark-if in XmlParser::parse() is inconsistent. Or, 
if compared to the rest of the file, it's the only consistent one in that 
function.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Information regarding upcoming Gitlab Migration

2020-04-27 Thread Rolf Eike Beer

Bhushan Shah wrote:


I've worked on draft "move" of the current set of the repositories in
their respective subgroups at the repo-metadata project's branch [1].
You can browse the directory structure to get idea of how final
structure on Gitlab would look like.


No objection, just a request for clarification: it looks like everything 
in extragear or playground was merged into there, right?


Eike


Re: KDiff3 1.8 release.

2019-03-19 Thread Eike
Hi!

Am Samstag, 16. März 2019, 17:19:17 CET schrieben Sie:
> On Sat, Mar 16, 2019, 1:14 PM Jonathan Riddell  wrote:
> > I see Debian has a 1.7 release, where is that available?
> 
> If it's truly 1.7 its based off a partial kf5 port that was never released.
> Don't remember off hand who created. 1.7.90 would be an older version of my
> code not officially released.

I uploaded an untagged 1.7.90 version to Debian so it could still make it
in the next Debian release buster (which is supposed to get rid of QT4).
=> https://packages.debian.org/buster/kdiff3

> > Eike is the Debian packaging managed in a repo somewhere?  Are you
> > interested in making it part of the KDE Qt Debian team?

KDiff3 Debian packaging is managed all old-fashioned on my local PC
(for over 15 years now...). :)
I didn't yet engage in team packaging...

Ciao,
Eike


signature.asc
Description: This is a digitally signed message part.


Re: Installing qml caches

2018-06-07 Thread Rolf Eike Beer

Am 2018-06-01 13:10, schrieb Alexander Volkov:

Hi all,

It would be nice to install .qmlc files in addition to .qml files to
reduce start-up time of applications.
They are generated with qmlcachegen. For Qt 5.11:
qmlcachegen -o example.qmlc qxample.qml

Currently qml files are usually installed the following way:
install(DIRECTORY qml/ DESTINATION ${KDE_INSTALL_QMLDIR}/org/kde/kcm)

So this could be changed to something like
ecm_install_qml(qml org/kde/kcm)

I wonder whether someone already working on this or want to implement
such a function?
If not, I'll try to do it myself.


Just a quick note: the quick compiler macros provided by Qt 5.11.0 are 
broken for cross-compilation (QTBUG-68724).


Eike


Re: Symmy in kde-review

2017-12-04 Thread Rolf Eike Beer
Am Montag, 4. Dezember 2017, 22:14:14 schrieb Elvis Angelaccio:
> On lunedì 4 dicembre 2017 10:25:48 CET, Tomaz Canabrava wrote:
> > On Sun, Dec 3, 2017 at 11:14 PM, Albert Astals Cid <aa...@kde.org> wrote:
> > El dijous, 23 de novembre de 2017, a les 10:34:41 CET, Elvis Angelaccio va
> > 
> > escriure:
> >> Hi,
> >> symmy has been moved to kde-review for the usual review process.
> >> 
> >> It's a tiny frontend for the symmetric encryption functionality of GPG.
> >> It
> >> doesn't handle signing or public/private keys, as we already have kgpg or
> >> kleopatra for that.
> > 
> > How hard would be to make that functionality to kgpg (simple
> > encryption without public / private keys) instead of yet -
> > another tool to handle file encryption?
> 
> Not sure, perhaps Eike can better answer that (kgpg already does symmetric
> encryption).
> I chose to go with an external process (+ qgpgme) for technical reasons
> (basically, to not freeze the dolphin UI). Since this means we get a
> self-contained binary, it can as well go in its own repo imho.

Right click on a file, Actions, Encrypt File, click Options, chose "symmetric 
encryption". All in a different process, so Dolphin is not blocked. If it's 
really an issue adding an own option to trigger this through their own action 
from the context menu.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Simple Menu (Plasma widget) now in kdereview

2017-11-02 Thread Eike Hein

On 11/02/2017 07:33 PM, David Edmundson wrote:
> ​What's the intended destination after review?

Honestly, no idea. Playground? Extragear, does that still exist? Where
do we put random projects these days? :)

I don't want it to be in kdeplasma-addons: That would be one menu too
many, and this is intended to be distributed mainly through the Store.

My main purpose in going through kdereview is to get Phabricator and
Bugzilla entries for it, because users are asking for a better way to
submit bug reports than Store comments.


> David

Cheers,
Eike




Simple Menu (Plasma widget) now in kdereview

2017-11-02 Thread Eike Hein

Hi,

I'd like to submit the "Simple Menu" widget for Plasma to kdereview.

Sysadmin kindly moved the repo in position for me:

https://cgit.kde.org/plasma-simplemenu.git/

A shiny website is found here:

https://store.kde.org/p/1169537/

And here is an old announcement blog:

https://blogs.kde.org/2017/01/30/simple-menu-launcher-kde-store

The Simple Menu codebase is QML-only, relying on C++ backends shipped
with Plasma Desktop.

It's had multiple tagged releases and is shipped as the default menu
in several distros.


Cheers,
Eike


Re: Review Request 130193: [cmake]: De-duplicate "else" to fix build with cmake-3.9

2017-07-20 Thread Rolf Eike Beer

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130193/#review103463
---


Ship it!




While at it, please also remove the arguments from the else() and endif() calls.

- Rolf Eike Beer


On Juli 20, 2017, 7:59 nachm., Heiko Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130193/
> ---
> 
> (Updated Juli 20, 2017, 7:59 nachm.)
> 
> 
> Review request for kdelibs and Frank Osterfeld.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> ---
> 
> Otherwise it errors out with:
> "CMake Error at kdeui/CMakeLists.txt:316 (else): A duplicate ELSE
> command was found inside an IF block."
> Also adjust indentation to match the surrounding lines.
> 
> 
> Diffs
> -
> 
>   kdeui/CMakeLists.txt d6ec8b47e9af686441ab5ab761be9ff424cbb556 
> 
> Diff: https://git.reviewboard.kde.org/r/130193/diff/
> 
> 
> Testing
> ---
> 
> Builds fine with cmake-3.9.0.
> 
> 
> Thanks,
> 
> Heiko Becker
> 
>



Re: kdereview: ksmtp

2017-05-16 Thread Rolf Eike Beer
Am Donnerstag, 11. Mai 2017, 17:03:01 schrieb Daniel Vrátil:
> Hi,
> 
> please review ksmtp, which is now in kdereview.

-the CMakeLists.txt has a mix of spaces inside () or not

-in loginjob, line 173, you check for code 25. Should this be 250? Or is that 
25*? Where is ServerResponse actually defined, I only see the header.

-does that support pipelining? I don't see any sync points, so I guess not.

-there is a longstanding bug in KMail that it violates the RfC when it has a 
problem with authentication (e.g. password rejected), that is does not 
properly QUIT the SMTP session, but just closes the socket. Is that properly 
handled?

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Draft Policy: Dependency Changes and CI

2017-01-23 Thread Eike Hein


On 01/22/2017 10:58 AM, Sandro Knauß wrote:
> For me the whole text sounds to bureaucratic and too many "you can do this 
> after waiting that long" .

I hear you, but we just had a major blow-up about this topic where
people got extremely agitated and threatened to walk out. The hoped
benefit of specifying the details in a policy doc is that it gives
people a clear idea of what is expected of them and what they are in
their right to do and when, which ideally minimizes opportunities
for blaming each other instead of blaming the doc.

If you go back to the original discussion, a central problem was
that sysadmin wants advance notice, but it wasn't clear how much
advance notice is good enough, and devs had concerns about too
long time windows in context of projects with short release
cycles, etc. - this was a major bone of contention, so I feel the
doc had to make an attempt to submit a compromise. The doc tries
to make a compromise between what people felt in their right to do
and reasonable.

Note that this document isn't necessarily set in stone for all time:
We try to live with it for some time, and if it turns out we can't,
we kick off another discussion process to refine or replace it. I
expect that people understand a fresh policy needs a trial period
before killing each other over it. I also expect people to make a
honest effort to try and follow it, though.

Bureaucracy can be a nuisance. But it can also slow things down
when needed, and dissipate strong emotions by turning them away
from people.


Cheers,
Eike



Draft Policy: Dependency Changes and CI

2017-01-19 Thread Eike Hein

Hi devs,

this month, a heated exchange on kde-core-devel about a dep
change breaking CI has moved us to draft a new policy to better
handle situations like this in the future.

While not "law", the policy _strongly_ suggests some guidelines
forming a quid-pro-pro protocol between developers and sysadmin
when it comes to making software dependency changes that impact
provisioning of the CI system. It should leave everyone with a
solid idea of what is expected of them and how to behave around
dependency changes.

Essentially, it requires developers to put sysadmin in the loop
when pre-announcing dep changes, giving them some time to ready
the CI system environment, and giving them an opportunity to ask
for more time when needed, while leaving maintainers/teams in
ultimate control over making the change. Meanwhile, sysadmin
does its best to procure new deps or communicate if/why it can't.
Worst case scenarios/failure modes and when and where to escalate
special cases are specified as well.

The draft is located here:

https://community.kde.org/Policies/Dependency_Changes_and_CI

While it's not too late to raise objections and comments, please
only do so after reviewing the kde-core-devel thread that lead
to its creation to see how it stands in the big picture. A
sizable number have already deliberated this draft. It's con-
structive to assume they did so with care.

The thread:

https://marc.info/?t=14836060552=1=2

The current plan calls for linking the new policy from the
/Policies/ overview page at the start of February, thereby
making it official and putting it into action. And then we
live with it for some time and see if it serves us well
going forward.


Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-19 Thread Eike Hein


On 01/19/2017 04:55 PM, Ben Cooksley wrote:
> It's already been fixed - by having the CI build libxkbcommon. It was
> also required to build an updated version of xorg-macros upon which
> that depended.
> Libraries that depend on KWindowSystem will have the self-built
> libxkbcommon made available to them.
> 
> Implementing this also required changes to the general CI
> infrastructure for all jobs, to handhold Autotools based builds. We
> did something similar for CMake already.

Thanks Ben!


> Regards,
> Ben

Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-19 Thread Eike Hein

On 01/19/2017 03:59 AM, Friedrich W. H. Kossebau wrote:
> Given this is a policy affecting all of the KDE projects, please first 
> propose 
> it officially in a separate own thread, with a proper subject. Perhaps even 
> on 
> kde-community, as kde-core-devel might not be read by many, given it used to 
> be focussed on kdelibs/core apps. Even people reading kde-core-devel might 
> have missed it, as this thread here started to become heated and long, so 
> most 
> free time contributors might not have invested time into reading more than 
> the 
> first.

Agreed - I'll be writing a separate mail to k-c-d and kde-devel very
soon. More communication is almost always better!


> Some feedback on the policy itself:
> "Dependency changes for software covered by the CI system should be submitted 
> through code review"
> -> I would propose to additionally recommend contacting sysadmin as soon as 
> one knows that a new dependency is coming up, not only first at the time the 
> official review request is made. That should give sysadmin some more time in 
> advance to handle the needed new dependency in CI, and perhaps also give 
> feedback on issues. 
> Ideally it might even result in the new dependency already being available at 
> the time of the review request, so if the review itself is done quickly, CI 
> does not block the merge.
> 
> IIRC this would also reflect what has been done now and then :)

Sounds good, I'll add a small bit.


> Cheers
> Friedrich

Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-18 Thread Eike Hein


On 01/17/2017 11:46 PM, Adriaan de Groot wrote:
> But CI has a really important function: it shows us the health of the sources 
> for everything; and that's something the release team needs, and the whole 
> community can be interested in. So "opting out" of CI deprives us of a good 
> view of the state of our software products.

Agreed. But under the proposed document, you can essentially only
opt out by behaving so badly that sysadmin sees no choice but to
kick you out, and it labels that as "rude". I think it also
communicates why we care about CI (e.g. as regression catcher).

This thread has slowed down now - there's been no strong objections
raised to the current version of the doc. If everyone is happy with
it, I propose we start linking it from the /Policies/ main page by
start of February and try to live with it.

As for the current situation with xkbcommon, it happened before
we sat down to write this, and I'd say we don't try to shoehorn
it into the framework after the fact. AIUI it sysadmin/packagers
are working to procure the needed dependency, so we'll let them
do so ...


Cheers,
Eike



Re: CI Requirements - Lessons Not Learnt?

2017-01-16 Thread Eike Hein

New draft is up, all changes are to the last section:

* I changed the overall tone from law into strongly suggested
  guidelines, with the central idea that sysadmin will only
  promise to keep up its end for projects which keep up theirs,
  i.e. a reasonable quid-pro-quo hrotocol witj properly codified
  parameters.

* I rephrased the conclusion based on the c-should-be-a feed-
  back and to back the above.

* I added a special exceptions clause, but it feels a bit
  wishy-washy to me. At least it integrates into our greater
  policy framework, though.

* I mentioned the "Sysadmin" group on Phabricator.

Please have a look.


Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-16 Thread Eike Hein


On 01/16/2017 06:58 AM, Alexander Neundorf wrote:
> - OTOH, if you are maintaining kwin fulltime as paid job, I consider it 
> reasonable to expect that the maintainer is able to maintain necessary 
> #ifdefs, or apply pragmatic solutions, just to solve the problem for his 
> users... but that is not realistic to expect if the maintainer is quite 
> stressed out and all alone with this work.

The maintainer's job is to ensure the continued health of the project,
which includes making sure it can move forward instead of getting
bogged down in supporting legacy stacks and a multitude of compilation
paths. It's entirely within the maintainer's (and by extension team's)
purview not to want to run their project that way, and it doesn't need
to be overworked or stressed out to do that.

There's always going to be a range of opinions on what window of
support for legacy dependencies is appropriate or not. Partly because
different windows make sense for different projects, different teams
and different stacks. It makes sense to trust maintainers/teams to have
the necessary experience and knowledge to make those calls, instead of
trying to codify a KDE-wide dependency policy here. Changes like this
increase the barrier to participation in KDE (more rules to abide by,
in this case, concretely more work to do that not all teams can
equally afford to do).


Cheers,
Eike





Re: CI Requirements - Lessons Not Learnt?

2017-01-16 Thread Eike Hein

I'll use this as a launching-off point to continue the debate:

Coming out of the weekend, after reflecting a little, I no longer
feel like the extremely hard approach I took in the current draft
is appropriate for the community at large, in line with ade's
concerns of over-democratizing. I don't think the use of "manda-
tory" is ultimately realistic for the bulk of KDE.org projects
(only a small number has developers who put in 40h/week), the
effects of trying to enforce these rules would be far more dama-
ging to the community than the gain, and it's important to remem-
ber that most KDE.org developers were never polled on whether or
not to use CI in the first place.

I'll be working up a new draft today taking some of the comments
so far into account, and giving sysadmin the latitude to remove
projects from CI at their decision if the guidelines are violated
and maintaining a project on CI becomes unreasonable. This limits
scope of enforcement (i.e. the consequences for falling out of
line) to participation in CI instead of the community, which
seems more pragmatic in hindsight.


Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-13 Thread Eike Hein
On January 14, 2017 1:23:20 AM GMT+09:00, "Martin Gräßlin" <mgraess...@kde.org> 
wrote:
>Am 2017-01-13 13:21, schrieb Eike Hein:
>> Ok, here we go. My draft of a formal policy for dep changes and the
>CI:
>> 
>> https://community.kde.org/Policies/Dependency_Changes_and_CI
>> 
>> Compared to my earlier email, this draft contains some hard deadlines
>> and an attempt to specify failure modes if the deadlines are not met.
>> 
>> Please chime in with suggestions for how the text needs to be refined
>> and expanded to meet your and our needs. Updated versions of specific
>> paragraphs are the preferred format for doing so: The thread so far
>> has shown that free-form conversation is prone to mudslinging, so
>> let's try to keep to the lingo fo a formal, dry document.
>
>Thanks for stepping up to write this!
>
>A few notes from my side:
>
>* "Subscribing the sysadmin team to these code reviews is mandatory." -
>
>How? What are the team names one has to add as reviewers?
>
>* This drastically changes the way KDE works. It requires mandatory
>code 
>review and gives kind of veto power to sysadmins. It's something the 
>larger KDE community might need to discuss as it removes one of the
>core 
>principles of KDE that anybody can commit to anything and code review
>is 
>only optional.
>
>* Related to this: a month blocking on formal process is too long. If a
>
>maintainer doesn't respond in two weeks but another team member
>accepted 
>the patch, it should also go in.
>
>* I would like to see a link to where developers can check whether a 
>dependency is available. Reasoning: I want to check whether it's a 
>no-brainer to not have to add sysadmins if it's already available. E.g.
>
>if I add a new dep in KWin, which is already used by Krita I wouldn't 
>know that and ask sysadmins. That would be a waste of sysadmin's time.
>
>* I would like to add another exception: last minute dependency
>requests 
>prior to a feature freeze should be allowed under certain conditions 
>even if sysadmins had not two weeks to respond. Reasoning: shit happens
>
>;-)
>
>Cheers
>Martin

Crappy mobile mail client, apologies for bad (rather no) quoting.

First up: No objections to swapping (a) and (c).

ade's "do we really have that much bureaucracy" and highlighting this codifies 
mandatory code review also resonate with me. I might make an alternate draft 
that takes the foot of the pedal a bit, depending on how the discussion evolves.

How to subscribe sysadmin: Indeed something we still need to figure out.

"Team member" instead of "maintainer": Agreed. We've done a bad job of tracking 
maintainers anyway, and that's partially because we have somewhat 
intentionally-weak roles, to encourage people to step up and exercise common 
ownership. We probably don't need make maintainers more special in this doc and 
can rely on them being consulted naturally as per usual. This also unburdens 
the bureaucracy slightly already.

Dep checking: Sysadmin will have to chime in there.

Special exceptions: I can try to come up with something.

I'm off to bed in this timezone. I'll see if I can revise during the weekend, 
otherwise Monday.


Cheers and thanks for good feedback so far,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-13 Thread Eike Hein

Quick follow-up notes: I improved the formatting a little more,
please refresh if you were reading already - now waiting for
comments, though.


Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-13 Thread Eike Hein

Ok, here we go. My draft of a formal policy for dep changes and the CI:

https://community.kde.org/Policies/Dependency_Changes_and_CI

Compared to my earlier email, this draft contains some hard deadlines
and an attempt to specify failure modes if the deadlines are not met.

Please chime in with suggestions for how the text needs to be refined
and expanded to meet your and our needs. Updated versions of specific
paragraphs are the preferred format for doing so: The thread so far
has shown that free-form conversation is prone to mudslinging, so
let's try to keep to the lingo fo a formal, dry document.


Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-13 Thread Eike Hein

> Sure, every time I need a new dep I'm going to do a bikeshed discussion like 
> this one. That was the last time I'm going through this. I can do better 
> things with my time. No, thank you. After the experience in this thread I can 
> only advise everybody to never ever tell the larger KDE community that you 
> are going to update the dep.

That's why we should work out a plan and a compromise for the future,
so we /don't/ need to go through it again, from either side.

New deps are going to continue to happen.

CI is going to continue to break.

Package procurement and scheduling is going to continue to be
difficult and frustrating.

This problem complex has no perfect solutions. What we can do and
should do is come up with a policy that sets realistic expectations,
so it's a known problem with associated known behavior guidelines,
that doesn't surprise anyone, that doesn't lead to a blame-game and
stops us from cannibalizing each other.

To that extend I'm going to put the policy I suggested in the thread
on a wiki page later, as a draft, and then I expect concrete notes
and suggestions for refining that draft, and I'll try my best to
moderate that process since I'm already a k-c-d list mod, and both
a Plasma dev and a (albeit largely inactive) sysadmin.


Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-12 Thread Eike Hein


On 01/13/2017 02:07 AM, Martin Gräßlin wrote:
> Fedora devs don't want it.

Since it might help: Kevin Kofler isn't part of the Fedora KDE SIG, so
his opinion in the thread is his own rather than representative of that
team/community.


> Martin

Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-12 Thread Eike Hein


On 01/13/2017 01:13 AM, Luca Beltrame wrote:
> Remember: even if we have disagreements (even strong), we're all on the
> same side.

I strongly agree with this. We have enough problems (e.g. making
great software) without fighting each other. As a bystander I've
been disappointed by multiple parties in this ordeal for being
confrontational completely needlessly instead of having the
maturity to step back, take in the big picture of the problem,
and try to work out a solution together. This yelling-at-the-
other-side style (and even threats of shutting down constructive
debate with taking steps) causes people to burn out, to quit to
save face, to say things in response they can't take back, and so
on. Avoiding these things is much more important than CI breaking.

Unhappy contributors are just as much an impediment to the quality
of our software. Don't break our software.

We're having major fight /because the build broke briefly over a
new dependency/. That's like setting the factory on fire because
a can of paint got knocked over. This is nuts.

You have the option not to fight.


Cheers,
Eike


Re: CI Requirements - Lessons Not Learnt?

2017-01-11 Thread Eike Hein

So my view of the situation is that we have a bunch of desires and
needs, and we need to sort them. Here's a couple I can reasily
identify:

* We want to have a working CI that gives us useful results

* We want to have developer flexibility in terms of using and catering
  latest dependencies

* We want the master branches to be realistically buildable by other
  devs

* We want to keep quality of HEAD high

These desires/needs are in conflict with each other, so it's not an
entirely simple sorting process. For example, "keep quality of HEAD
high" can require dragging in very recenty dependencies when our
and the other code need to act in concern to address a problem. On
the other hand, "want to have a working CI" and "realistically
buildable" means avoiding silly-recent dependencies that are hard
to procure.

I submit that one of the guidelines of our dev process it that
regression-type defects take priority over newly-identified defect
Breaking CI or breaking developer builds is a regression. Changes
that require new deps (usually) aren't related to regressions, un-
less the external software update is about fixing a regression. I
think local (our) regressions outweigh external regressions.

If you accept this sorting, then my proposal is:

1. Make it mandatory to file a review request for dependency changes
2. Make it mandatory to subscribe sysadmin to those reviews and get
their input
3. Sysadmin commits to providing that input in a reasonable time frame
4. Sysadmin tries to procure the new dependency in a reasonable time frame

This puts veto power into the sysadmin's hands, which is what we
want under the proposed sorting (we want a working CI and this is
how we get it).

Note that alternatives to this are practically difficult, unless
developers are willig to become sysadmins and/or packagers.


Cheers,
Eike



Re: Review Request 128941: ZLIB dependency is in libkonq since 7635179

2016-09-18 Thread Rolf Eike Beer

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128941/#review99266
---




konqueror/src/CMakeLists.txt 
<https://git.reviewboard.kde.org/r/128941/#comment66843>

If Konqueror directly uses these symbols it needs to explicitely link to 
it, even if libkonq does.


- Rolf Eike Beer


On Sept. 18, 2016, 11:36 nachm., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128941/
> ---
> 
> (Updated Sept. 18, 2016, 11:36 nachm.)
> 
> 
> Review request for KDE Base Apps and David Faure.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> ---
> 
> Add ECMMarkAsTest, used by fsview tests
> 
> 
> Diffs
> -
> 
>   konqueror/CMakeLists.txt 53c4829cbc2f8b380ad8608b555eb6e15b24a3bb 
>   konqueror/src/CMakeLists.txt e8e408611335fa56faf24466307f83e40a3b70ee 
>   lib/konq/CMakeLists.txt 7a61493ff13561340c1a6c114763489343212f41 
> 
> Diff: https://git.reviewboard.kde.org/r/128941/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>



Re: kconfig_compiler help ( reproducible builds )

2016-05-27 Thread Rolf Eike Beer
Am Freitag, 27. Mai 2016, 09:18:25 schrieb Scarlett Clark:
> I still need help with this, hacking the packaging was not accepted.
> Here is the exact problem:
> https://reproducible-builds.org/docs/locales/#collation-order
> 
> Looking at kconfig_compiler code that only thing that is standing out is:
> 
> https://github.com/KDE/kdelibs/blob/23f0972e03d9d5f30412b7c9c74cb4cadef7293a
> /kdecore/kconfig_compiler/kconfig_compiler.cpp#L481
> 
> 
> Am I on the right track? If so can someone with more C++ knowledge help me
> with a solution?

Is this actually outputting UTF8 sequences? I would expect a call to .toUtf8() 
somewhere in there.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: A new attempt on PyKDE5 binding generation

2016-03-27 Thread Eike Hein


Hi,

exciting work! Just the other day I was hoping someone would pick
up work on PyKDE5 ...

Cheers,
Eike


Re: A new attempt on PyKDE5 binding generation

2016-03-27 Thread Eike Hein


Hi,

exciting work! Just the other day I was hoping for someone
to take up PyKDE5 ...


Cheers,
Eike


Re: Review Request 104960: KJS: fix behaviour on allocation errors

2016-02-07 Thread Rolf Eike Beer

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/104960/
---

(Updated Feb. 7, 2016, 5:29 nachm.)


Status
--

This change has been discarded.


Review request for kdelibs.


Repository: kdelibs


Description
---

The KJS allocator will likely crash with a 0-deref on allocation errors. The 
exact behaviour will also depend on the platform, e.g. a Un*x platform without 
posix_memalign() will have MAP_FAILED as the pointer used for calculations 
(which is (void*)-1), other will have 0.

This will make the allocator have a sane default behaviour: just return 0.


Diffs
-

  kjs/collector.cpp 70e4757 

Diff: https://git.reviewboard.kde.org/r/104960/diff/


Testing
---


Thanks,

Rolf Eike Beer



Re: Change to Mail Infrastructure - SPF and DKIM verification will now be enforced

2015-12-04 Thread Rolf Eike Beer

Am 04.12.2015 11:08, schrieb Ben Cooksley:
On Fri, Dec 4, 2015 at 9:01 AM, Rolf Eike Beer 
<k...@opensource.sf-tec.de> wrote:


Think of SPF: I sent an email to a kde.org email address only some 
weeks ago.
My domain sets a SPF policy. The KDE server accepts this (it's 
actually
correct), and then sends the mail on (unaltered). Now the next server 
also
checks SPF and will reject the mail, because the KDE server is not 
allowed to
send mail for my domain. Now you have 2 ways out: either the KDE 
server
rewrites the "mail from" header (what you will later find as 
Return-Path in the
mail header), or the final destination says allows the user to say 
"hey, I use
those kde.org server as a forwarder to me, so whatever SPF says, mails 
from
that host are fine". Both ways work, both are fine, but both require 
some sort

of action somewhere on the path.


Rewriting to workaround SPF restriction is also standardised - as a
mechanism known as SRS - see http://www.openspf.org/SRS


Has KDE implemented this in the last few weeks? Before it was not.


That part is simple. For DKIM stuff get's more complicated because you
sometimes _have_ to modify the body, e.g. when you need to base64- or 
qp-
recode parts of the mail because the receiving mail server does not 
support
8bit-transfer (which is an issue by itself, but still sadly legal). So 
with
DKIM you are actually screwed at this point. The only good way it is 
again to
permit your users to ignore DKIM signatures from certain hosts (e.g. 
if you
subscribe to a Debian list, then simply ignore DKIM for the Debian 
servers).

Finding out those itself is not an easy task either.

So all in all one can enable DKIM for list services, but for user 
accounts it
should be opt-in with an easy way to whitelist certain hosts for 
relaying.

Everything else is just asking for endless bounces.


Note that DKIM senders and receivers are usually running on modern
infrastructures, so 8bit transfer shouldn't be an issue.
For user to user transmission, there is no reason why mail bodies
would be modified.


Well, nice try. Out of 5 mail providers I checked 3 failed: AOL, GMX.de, 
Web.de.


Greetings,

Eike


Re: Change to Mail Infrastructure - SPF and DKIM verification will now be enforced

2015-12-04 Thread Rolf Eike Beer
Ben Cooksley wrote:
> On Fri, Dec 4, 2015 at 11:28 PM, Jan Kundrát <j...@kde.org> wrote:
> > On Friday, 4 December 2015 10:56:42 CET, Ben Cooksley wrote:
> >> Note that in the long run with DMARC looming you will need to switch
> >> to #2 anyway, and keeping your current behaviour will likely lead to
> >> mail from people who use Yahoo / AOL / etc ending up in the spam
> >> folder with many mailing list members. I'll be starting a discussion
> >> regarding taking this step on KDE systems at some point in the near
> >> future (switching to DMARC compatible policies).
> >> 
> >> For more information, please see http://wiki.list.org/DEV/DMARC
> > 
> > Do I understand your plan correctly? The following projects appear to not
> > re-sign their ML traffic, and they mangle headers at the same time. If I
> > understand your plan correctly, this means that I won't be able to use my
> > @kde.org addresses on mailing lists of these projects, for example:
> > 
> > - Qt,
> > - Debian,
> > - Gentoo,
> > - OpenStack,
> > - anything hosted at SourceForge,
> > - and many, many more, essentially anybody who were ignoring DKIM.
> > 
> > Please, change your plans, this is obviously a huge no-go.
> 
> *Sigh*.
> 
> Debian has already committed (prior to any of this) to making their
> mailing lists DMARC compliant by ceasing the alteration of mail
> passing through their lists.

Which is a good idea anyway, as far as you can influence it (see the 8bit 
problems from the other mail).

> It is an extreme pity these mailing list providers have demonstrated
> such an extreme disregard for standards which aim to eliminate
> forgeries and ensure people cannot be digitally misrepresented. This
> is why we had to change Bugzilla a while back to send as
> bugzilla_nore...@kde.org instead of the acting user's email address -
> because Yahoo's enforcement policy meant GMail always placed mail from
> Yahoo users in the spam folder.

Huh? Of course you _must_ send with a @kde.org address. My SPF policy forbids 
you to send mail for my domain. And now you want to enforce SPF, but don't 
properly handle it yourself?

> I'll grant an extension until 31 January, however I would like to see
> commitments from some of these to improve their infrastructure.

It wont affect me, as I ignore the whole DKIM stuff both at the sending and 
receiving end, but this just going to cause a lot of unnecessary trouble I 
think.

To make it clear: I receive tons of spam per day. It has become worse in the 
last month, as it seems that the usual filters do not work that good anymore. 
You as postmaster of such a public domain are likely receiving even more of 
that crap. But that proposal is going to cause collateral damage.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Change to Mail Infrastructure - SPF and DKIM verification will now be enforced

2015-12-03 Thread Rolf Eike Beer
Am Donnerstag, 3. Dezember 2015, 11:54:43 schrieb Jan Kundrát:
> On Thursday, 3 December 2015 07:13:07 CET, Ben Cooksley wrote:
> > I will be re-enabling DKIM validation in one week's time - which will
> > then break subscriptions to Debian mailing lists (as any email from
> > anyone who has enabled DKIM which hits their lists will not be
> > accepted by KDE email infrastructure)
> 
> Ben, could you please briefly explain your idea about how a complying
> mailing list service should behave? Suppose that I have an installation of
> mlmmj which:
> 
> - mangles the Subject header,
> - preserves the original From header,
> - maybe replaces a Reply-To with the ML's address,
> - introduces a bunch of specific List-* headers,
> - otherwsie doesn't manipulate the MIME tree or the message texts.
> 
> What should I do to make sure that this service continues working once you
> flip the switch?

This is actually a flaw in those standards: they think that everyone you 
communicate with will comply. If you as a mailing list service do not then 
everyone that sends to your list will eventually get in trouble if you do not 
rewrite the message or resigns it.

Think of SPF: I sent an email to a kde.org email address only some weeks ago. 
My domain sets a SPF policy. The KDE server accepts this (it's actually 
correct), and then sends the mail on (unaltered). Now the next server also 
checks SPF and will reject the mail, because the KDE server is not allowed to 
send mail for my domain. Now you have 2 ways out: either the KDE server 
rewrites the "mail from" header (what you will later find as Return-Path in the 
mail header), or the final destination says allows the user to say "hey, I use 
those kde.org server as a forwarder to me, so whatever SPF says, mails from 
that host are fine". Both ways work, both are fine, but both require some sort 
of action somewhere on the path.

That part is simple. For DKIM stuff get's more complicated because you 
sometimes _have_ to modify the body, e.g. when you need to base64- or qp-
recode parts of the mail because the receiving mail server does not support 
8bit-transfer (which is an issue by itself, but still sadly legal). So with 
DKIM you are actually screwed at this point. The only good way it is again to 
permit your users to ignore DKIM signatures from certain hosts (e.g. if you 
subscribe to a Debian list, then simply ignore DKIM for the Debian servers). 
Finding out those itself is not an easy task either.

So all in all one can enable DKIM for list services, but for user accounts it 
should be opt-in with an easy way to whitelist certain hosts for relaying. 
Everything else is just asking for endless bounces.

> I would like to have more information about what you mean by "DKIM
> validation" -- what specific steps are you going to introduce, and how is
> the end result going to react to a missing or invalid DKIM signatures.
> 
> Also, quoting RFC 6376, section 6.3:
> 
>In general, modules that consume DKIM verification output SHOULD NOT
>determine message acceptability based solely on a lack of any
>signature or on an unverifiable signature; such rejection would cause
>severe interoperability problems.  If an MTA does wish to reject such
>messages during an SMTP session (for example, when communicating with
>a peer who, by prior agreement, agrees to only send signed messages),
>and a signature is missing or does not verify, the handling MTA
>SHOULD use a 550/5.7.x reply code.
> 
> That seems in line with what e.g. GMail is doing, only enforcing DKIM
> validation for notoriously faked domains like eBay and PayPal where the
> phishing potential is high.

No, this means that a mail should not be rejected if there is _no_ signature, 
or a malformed one. If a domain publishs that it does DKIM signing and mails 
are expected to be correctly signed, then rejecting on a invalid signature is 
actually fine (and the sole purpose of this RfC).

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Spectacle moved to KDE Graphics, future of KSnapshot?

2015-09-25 Thread Eike Hein


On 09/24/2015 12:11 PM, Boudhayan Gupta wrote:
> Now, what'll come of KSnapshot? Spectacle clearly obsoletes KSnapshot
> on X11 (and Wayland as long as KWin is running - if a generic Wayland
> protocol isn't out by dependency freeze I'll add a generic KWin
> backend temporarily for this release). However, we have no Mac OS and
> Windows backends, so KSnapshot clearly still has use in those
> platforms.

I'm more concerned about the migration path from KSnapshot
to Spectacle. Can we make a hard decision to abandon
KSnapshot on X11 and have Spectacle install a ksnapshot
symlink for a grace period? Yes it means a file conflict
with a ksnapshot package, but that's the "hard decision"
part. And if Spectacle is launched through the symlink it
should throw up an info type KMessageWidget with explanation.


> What about moving KSnapshot to Extragear?
> 
> Cheers,
> Boudhayan Gupta

Cheers,
Eike


Re: Review Request 124824: [OS X] FindKDE4Internal.cmake : reintroduce a cmake_minimum_required statement

2015-08-19 Thread Rolf Eike Beer
Am Mittwoch, 19. August 2015, 21:14:38 schrieb Heiko Becker:
 I stumbled upon the same, it's actually a bug in cmake fixed by this commit:
 
 http://www.cmake.org/gitweb?p=cmake.git;a=commit;h=b9ec9392da21a3421e48c6961
 976060d872faffb

But the short fix for this is indeed to put cmake_minimum_required() into your 
project. But not into a project you include, but into the main project you are 
trying to build.

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.


Re: KScreenGenie moved to KDE Review

2015-06-28 Thread Eike Hein


On 06/28/2015 11:10 AM, Martin Gräßlin wrote:
 In opposite KSnapshot is a very mature application with a decade 
 of development behind it.

It's also a rather stagnant one, and a new developer with
the desire to improve upon the status quo is a high-value
community asset much in the same way as a mature application
can be, and both should be treated with equal care.

One way to have our cake and eat it, too, i.e. assure that
users receive a quality even if new product from us, is to
do as asked and participate in the review process. I pointed
out a few small bugs on IRC recently, but overall walked
away with a favorable impression of the code.


 Cheers
 Martin

Cheers,
Eike


Re: Distros and QtWebEngine

2015-04-21 Thread Eike Hein



On 04/20/2015 08:17 PM, Albert Astals Cid wrote:

IMHO the duty of a distro is providing software to their users to use


But not under any circumstance: Enforcing some level of hygiene and
quality in the software provided is a service rendered in my interest
and protects me as a user. A lot of good has come from Debian forcing
projects to play nice with others.




Cheers,
   Albert


Cheers,
Eike


Re: The Future or KDE PIM Releases

2015-04-13 Thread Eike Hein



On 04/13/2015 03:04 PM, Sebastian Kügler wrote:

This sounds awesome.


+1 - just speaking as a user even, that's great news.


Cheers,
Eike


Re: Review Request 122987: Allow user to specify path to myspell dictionary files

2015-03-17 Thread Rolf Eike Beer


 On März 17, 2015, 1:07 nachm., Laurent Montel wrote:
  src/plugins/hunspell/hunspellclient.cpp, line 27
  https://git.reviewboard.kde.org/r/122987/diff/3/?file=355372#file355372line27
 
  #include ...
  we use local file.

No, the file is in an include path, not in the same directory as the source 
file (binary vs. source dir).


- Rolf Eike


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122987/#review77630
---


On März 17, 2015, 1:09 nachm., Eugene Shalygin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122987/
 ---
 
 (Updated März 17, 2015, 1:09 nachm.)
 
 
 Review request for KDE Frameworks and kdelibs.
 
 
 Repository: sonnet
 
 
 Description
 ---
 
 Not on all systems myspell dictionaries are located in 
 /usr/share/myspell/dicts/,  which is hardcoded in the hunspell plugin 
 sources. For instance, on Gentoo it is /usr/share/myspell/. So let the user 
 define this path.
 
 
 Diffs
 -
 
   src/plugins/hunspell/CMakeLists.txt 098fb49 
   src/plugins/hunspell/config-hunspellplugin.h.cmake PRE-CREATION 
   src/plugins/hunspell/hunspellclient.cpp 9e3aa67 
   src/plugins/hunspell/hunspelldict.cpp 621113d 
 
 Diff: https://git.reviewboard.kde.org/r/122987/diff/
 
 
 Testing
 ---
 
 hunspell plugin began to work on Gentoo
 
 
 Thanks,
 
 Eugene Shalygin
 




Re: kio-extras

2015-02-24 Thread Eike Hein



On 01/22/2015 11:42 AM, Jonathan Riddell wrote:

kio-extras is currently released part of Plasma 5.  It's need said
that it would be better to be part of applications because they're
plugins used by applications and typically not by the desktop.


kio-extras contains the desktop:/ kioslave that Folder View
in Plasma Desktop defaults to.

(Perhaps it shouldn't.)




Jonathan


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-02-02 Thread Eike Hein



On 02/02/2015 01:20 PM, Milian Wolff wrote:

Sigh, I find it highly sad to read this over and over again. People keep
confusing the flaky CI and the high quality barrier in Qt with gerrit
itself... Seriously, gerrit the tool is OK, what makes it hard and what is the
actual barrier to entry in Qt are the flaky CI which kills productivity and
then sometimes the odd reviews with a pretty harsh tone that demand an
extraordinary quality without holding your hand much. But that's not related
to gerrit... These are different things, so please people - lets not confuse
these things and say using gerrit means using it exactly like Qt is using it!


1) I wrote contribute to Qt intentionally there,
   not use gerrit, because I'm aware of the diff.
   Although no, my unhappiness is not limited to the
   CI; many of my problems are actually linked to
   the way JIRA and gerrit have been made to inter-
   play for auth.

2) I've posted other criticizims of gerrit in the
   thread for a fuller picture of how gerrit adds
   to the unhappiness. Jumping on this bit, hand-
   waving it away and claiming we're now left with
   gerrit is OK is incorrect.



Bye


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-31 Thread Eike Hein



On 01/31/2015 08:37 PM, Christoph Feck wrote:

Excuse me, but if KDE developers will have to follow equivalent steps
as described at http://qt-project.org/wiki/Setting-up-Gerrit to
contribute, then I predict another big loss of developers.


This information could be pared down considerably and presented
much more nicely - and would have to be if we decide to use
gerrit. In addition, kdesrc-build could automate some of the
local setup (Qt's ./init-repository script does as well) to get
people started faster.

I agree the barrier-to-entry with gerrit is high, but I don't
consider that Qt page to be up to our standards if we were to
put serious thought behind communicating the use of gerrit.

Keep in mind though I've raged quite a bit about gerrit's UI
in this thread, and yes, this is part of it. I think gerrit's
useful, but I don't enjoy using it, personally.



Christoph Feck (kdepepo)


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-31 Thread Eike Hein



On 01/31/2015 08:57 PM, Alexander Neundorf wrote:

hmm, looking back at our switch to git, I don't consider our standards for
documentation of the developer workflow as very high unfortunately. :-/


Considering I wrote the majority of 
https://community.kde.org/Sysadmin/GitKdeOrgManual I guess I'll have to 
take that to heart, then ...


My vision for how to do a use gerrit page well would be to
lead with the minimal recipe for how to clone a project and
upload a change for review. It should put visual focus on
those couple of commands, and make explanation of what they
do secondary. It should also encourage the use of kdesrc-
build as an even quicker alternative. Most of the content
on the Qt page should only be much further down and less in-
your-face, or even on a secondary page, and framed as con-
venience tweaks useful when contributing regularly.

Basically similar to how programming languages or web frame-
works will structure their website these days, leading with
get you started info and putting effort into making that
a fast path.

It'd be nice to even make this recipe box a bit dynamic -
click something to get a project selector, pick the KDE
project, and get the commands shown adapted for copying
with the right repo URL inserted.

I think it's doable in principle, but I think the best-
case scenario is still to make gerrit be not a hassle on
coders; I think both its UI and the way you interact with
it have fundamental problems when it comes to non-code
contributors.

I don't think this is shifting of goal posts either be-
cause (a) ReviewBoard already has some screenshot functio-
nality and we actually have policy in place to require
showing screenshots for review requests that change UI
(i.e. this is something gerrit will actually regress us
on afaics) (b) the very reason we entertain shaking up
our infra is because we're unhappy with limitations in
our tools.



Alex


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-31 Thread Eike Hein



On 01/31/2015 10:37 AM, Jan Kundrát wrote:

About the we could vs. we will in general, I have to admit I'm
slightly confused by that. The proposal is careful to describe what is
available today, and to make a clear difference in saying what needs to
be done in future. Maybe some part needs clarification -- what parts do
you think are more of the yes-this-would-be-nice-but-I'm-worried nature?


I just wanted to make the point (but I agree I made it a bit
poorly) that it's worth differentiating between features the
proposed solutions are guaranteed to deliver right away, and
features they may enable in the future, but only if certain
other conditions are met, e.g. raw infrastructure expansion.

That's not a knock against your proposal, though - it's much
easier to e.g. ask for hardware donations when you can point
to a clearly identifyable need.

--- snip ---

I'd like to summarize my current feelings on both proposals.


Here's what I think gerrit's strong points are:

* There's undeniably synergy and cultural alignment with
  middleware communities we depend on to be had. Qt is
  using gerrit and we intend to remain a major stakeholder
  in Qt development, which means a sizable number of KDE
  developers need to be familiar with gerrit anyway. KDE
  using gerrit lowers the barrier for KDE developers to
  work upstream, and for folks in the wider Qt community
  to involve themselves in KDE. Particularly for Frame-
  works development this could be a boon.

* gerrit and the community around gerrit (which includes
  Jan) appears to have considerable chops when it comes
  to solving hard infrastructure problems like advanced
  CI and replication. This is stuff that can enhance the
  quality of our products by improving our processes,
  but it also stands to make KDE a more attractive envi-
  ronment for incubating new projects. Infrastructure
  should not be the sole platform we run recruitment on,
  but staying competitive is important, and one way to
  do that is to try and lift things only a community of
  KDE's size can lift -- and that can e.g. include
  gathering donations for fancy CI cluster setups.


Here's what I think Phabricator's strong point is:

* From what I've seen and played with so far, I really
  like the UI. This is a short bullet, but an important
  one.

* In terms of cultural alignment, I see communities like
  Blender and Wikimedia pick Phabricator. Lowering the
  barrier for cross-pollination with communities like
  this seems very attractive to me, because there's
  types of talent engaged in those communities that we
  need more of in KDE, e.g. designers and artists.

* I expect the above drivers to make it more likely that
  Phabricator will develop functionality that will make
  it more useful for these types of contributors. For
  example, good diff viewers for visual assets, good
  handling of screenshot attachments to change proposals,
  etc. I feel like it is less likely that gerrit will
  become similarly useful for non-code assets.

* The conflation of change review and issue tracking in
  Phabricator makes sense in the above context as well,
  and could greatly improve our non-programming develop-
  ment processes. In short, I don't see gerrit benefit
  our non-code contributors at all, but I see a shot at
  Phabricator doing so.

Given all of this, I find it really hard to have a firm
opinion at this point. Both proposals promise strong
benefits to different aspects of what the community is
doing, which makes this an exercise in weighing those
aspects and predicting future outcomes based on different
ratios between them.

That said, at this point, I have more experience work-
ing with gerrit than with Phabricator, and I think that's
something that needs to be addressed as part of this
process. I think we should proceed with setting up a test
instance and giving it a spin, this should help with
getting things into clearer focus.


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-31 Thread Eike Hein



On 01/31/2015 09:25 PM, Boudewijn Rempt wrote:

In short, Qt uses gerrit is a bogus argument in favor of gerrit.


The argument isn't so much that gerrit is working well
for Qt, but more that there's a certain simplicity in
using the same tooling across the KDE/Qt stack, and
that KDE benefits from having more KDE people active in
Qt. But I actually find contributing to Qt pretty frus-
trating (the infra is flaky, the process tends to break
down here and there, and I sometimes have to step out-
side gerrit and side-channel via email to get things
moving again), yeah.

It's one bullet from a long mail that addresses many
other points, though ... I agree that the KDE community
has concerns and ambitions unrelated to its relation-
ship with Qt and a broader activity scope than the Qt
project, and our tooling needs to be evaluated in that
broader context.



Boudewijn


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-31 Thread Eike Hein


... in fact, even if you consider Qt and KDE in symbiosis,
you could say that KDE is the place you can do things that
don't fit the narrower scope of Qt Project, and that calls
for tooling that supports things gerrit doesn't support
well enough. If gerrit is a constraint, then KDE picking
tooling other than gerrit would arguably contribute to the
combined ecosystem by making a resource available that is
otherwise lacking.


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-29 Thread Eike Hein



On 01/29/2015 06:16 PM, Milian Wolff wrote:

FWIW, this document reads like a fairy tale to me. The fact that so much is
already tested and deployed


One thing I'm unclear on: Does the gerrit test instance run
on machines administrated by kde.org these days?



Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-29 Thread Eike Hein



On 01/29/2015 06:51 PM, Jan Kundrát wrote:

The VM runs at my workplace. The KDE sysadmins have root access,
PostgreSQL backups are automatically pushed to a KDE server twice a day,
and Git is replicated to git.kde.org within seconds after each push.


Just for the record: I consider you a KDE sysadmin (you're
administrating infra used by KDE, after all), so I meant the
kde.org more general. Thanks.



Cheers,
Jan


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-29 Thread Eike Hein



On 01/29/2015 06:24 PM, Milian Wolff wrote:

Much nicer, I think!


I disagree - having the comment in a floating popup instead
of breaking up source code makes it easier to read the code
for me.

Personally, I agree that the gerrit UI is terrible to use.
It's not just the diff viewer, either. The general review
page layout is like someone spilled a jar of UI on the page.

Basic functionality like Reply is placed oddly, the history
view is useless-by-default by intially collapsing comments
and muddying up the excerpts with redundant preambles, ...
it's just really, really bad, and Phabricator blows it out
of the water.

For me, the strong suits of the gerrit proposal are:

* There's the psychological/branding advantage of using
  tooling that much of our middleware stack uses, which is
  a crowd we historically aren't culturally close to, at a
  cost to us. This might offset the cost of the daily frus-
  tration of having to work with shit UI.

  In this sense gerrit is the IBM Thinkpad of collabora-
  tion tools: It looks nerd. It's actively alienating to
  people who aren't willing to look past the surface,
  and make no mistake, that's a comforting veneer to many
  engineers.

  Habituating developers to numb themselves to living with
  shit UI seems kinda dangerous in context of the software
  we are trying to make, though.

* gerrit appears to solve some problems well that are no
  less hard than good UI design, like replication. Tapping
  into that work is definitely attractive.

* I like that Jan is looking ahead at using tooling to
  improve our process, rather than just re-ticking boxes
  for things we already have.



Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-29 Thread Eike Hein



On 01/29/2015 08:54 PM, Luca Beltrame wrote:

This is only the perspective of an occasional contributor, so perhaps it
doesn't weigh as much.


I think it's a real concern, and I'm wary of we can patch
it away because carrying a huge custom patch delta for UI
mods is what kept us from upgrading Bugzilla for multiple
years. I think is it realistic that we can maintain this
and keep up with upstream even if Ben or Jan get hit by a
bus is an important question with regard to both proposals.

I have similar concerns with some of the promised benefits
in the proposal because they strike me more of we could,
which is cool, but it's not we will. E.g. if test build-
ing precombined patches takes an OpenStack cluster - do we
have one? Where are we going to get that horsepower? Can
we keep it?

OTOH I expect that I *will* spend 1-2 hours a day five-six
days a week in the review web UI, and Phabricator's is
pretty nice off the shelf.

It's tricky.


Cheers,
Eike


Re: Another proposal for modernization of our infrastructure

2015-01-29 Thread Eike Hein



On 01/29/2015 10:34 PM, Thomas Lübking wrote:

Given the multiple concerns on the gerrit webfrontend (not only in this
kcd thread) I however also assume that it should be not too hard to get
a serious improvement upstream.
That includes If we endup w/ a -hypothetical- decision between
'powerful, but the webfrontend sucks' and 'pretty ui, but the backend
seriously falls short', i'd be happily willing to help on an improvement
here.

(At least from my POV, it should be simpler to fix some GUI than to get
a well scaling replication and CI backend - by the order of some
magnitudes ;-)


Maybe, but this is actually something I like from the
Phabricator proposal: It provides an impression of our
relationship with Phabricator upstream, which it says
is a good and constructive one.

In my experience, this is worth tons. Consider the
switch we did from autotools to CMake, where CMake
partly won out over other solutions because upstream
really wanted to work with us and did. We had similar
luck with gitolite, where upstream did a ton of real
feature work to address our needs. Redmine/Chili
helped us a little bit, too, actually.

So keep in mind that we need to fix this if we adopt
it takes manpower, and historically, we've relied to
some degree to getting that manpower *from upstream*.
This is is in my book a world of difference from
upstream might accept our patches ... maybe.


Cheers,
Eike


Apps 14.12 release aftermath / Running KF5 apps in KDE 4

2015-01-26 Thread Eike Hein


Hi,

it's becoming increasingly clear from feedback that we didn't
think the decision to ship KF5 apps in 14.12 through very well.

Distros are currently rolling it out as an upgrade to KDE 4
systems over the last 4.x apps, and users are running into the
following problems:

* KF5-based apps don't pick up visual settings from KDE 4 and
  can't be configured because System Settings 5 is usually not
  co-installable.

  I believe this needs to be addressed in the QPA plugin we
  ship with Frameworks and have added a new BKO product for
  the plugin as well as opened a ticket:

  https://bugs.kde.org/show_bug.cgi?id=343336

* Some apps apparently don't use the support class to migrate
  the app config file to the fd.o location, which means from
  the user POV they lose all settings for the app.

  Here we need to go through every app and check and fix it,
  and make sure we don't ship anything without that QA check
  again on future ports.

I cross-posted this to k-c-d and r-t for visibility - I suggest
we discuss the Frameworks side on k-c-d and the release QA side
on r-t.


Cheers,
Eike


Re: Sysadmin report on the modernization of our infrastructure

2015-01-21 Thread Eike Hein



On 01/21/2015 10:18 PM, Ivan Čukić wrote:

Hi,

I'm volunteering KActivities for the test.


Konversation, too.



Cheerio,
Ivan


Cheers,
Eike


Re: Sysadmin report on the modernization of our infrastructure

2015-01-21 Thread Eike Hein



On 01/21/2015 05:12 AM, Ben Cooksley wrote:

Hi all,

As promised in the earlier thread, i'd like to present the sysadmin
report on the state of the infrastructure surrounding our code.


As someone on the original git infra team, I'm impressed - thanks
for your hard work.




Cheers,
Ben Cooksley
KDE Sysadmin


Cheers,
Eike


Re: KDE4 SSL

2014-10-21 Thread Rolf Eike Beer
Am Dienstag 21 Oktober 2014, 10:52:38 schrieb Thiago Macieira:
 On Monday 20 October 2014 21:00:51 Pali Rohár wrote:
  Hello!
  
  Do you know which KDE4 libraries are using SSL and TLS protocols?
  And it is now possible to disable SSLv2 and SSLv3 protocols in
  KDE4 libraries?
 
 That would be QtNetwork, by way of kdecore and kio. SSLv2 is already
 disabled by default. To disable SSLv3 by default, you need to modify
 QtNetwork.

What happens if OpenSSL is built with no-ssl3?

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 120543: Update FindPostgreSQL.cmake

2014-10-10 Thread Rolf Eike Beer

Am 10.10.2014 08:46, schrieb Jaroslaw Staniek:
On 10 October 2014 08:05, Rolf Eike Beer k...@opensource.sf-tec.de 
wrote:
Update FindPostgreSQL.cmake to make is useful. Based on cmake's (3.x) 
one
but further improved PostgreSQL_TYPE_INCLUDE_DIR lookup. The fix 
comes from

libpredicate (master).


I see no upstream bug report for this.


Would a bug report for Calligra master be OK for you?
This is the only user of the PostgreSQL_TYPE_INCLUDE_DIR in entire KDE
I the know about:

http://lxr.kde.org/search?_filestring=_string=PostgreSQL_INCLUDE_DIR


You are not looking for PostgreSQL_TYPE_INCLUDE_DIR here, but for 
PostgreSQL_INCLUDE_DIR. And there seems to be no user of that at all 
inside KDE.



I am sorry if I misunderstood.

Good thing that the file disappears in KF5, since cmake has pretty
good own copy (not sufficient but I'll try to patch in the upstream).


CMake is the relevant upstream, Calligra is downstream (i.e. it uses our 
stuff).


Eike


Re: Review Request 120202: [OS X] improvements to the kwallet/OSX keychain integration

2014-09-21 Thread Rolf Eike Beer
 OK, implementation question.
 
 How do I declare a slot in a private class that doesn't have a specific
 header file? Putting `private QSLOT` above the function definition makes
 things compile, but the runtime complains about a missing slot (curiously
 even expecting it in KWallet::Wallet). Yes, I did update the connect call
 to pass in the pointer to the parent class 

Use Q_PRIVATE_SLOT in the public header?

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 120287: [OS X] make kde-workspace build

2014-09-20 Thread Rolf Eike Beer
Am Freitag, 19. September 2014, 22:57:40 schrieb René J.V. Bertin:
  On Sept. 20, 2014, 12:26 a.m., Christoph Feck wrote:
   You added APPLE to the if() but not always to the matching endif()...
 
 True. But that's optional, no?

The endif needs to have either a matching argument to the last if or elseif, 
or an entirely empty argument.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Using Gerrit for code review in KDE

2014-09-13 Thread Eike Hein



On 13.09.2014 17:49, Martin Gräßlin wrote:

my understanding was that it's still possible to bypass the code review, so I
cannot see that it's against the KDE Manifesto as it's only a kind of social
contract. Or am I missing something.

Personally I like the idea of having the +2 limited to the devs familiar with
the code.


I *strongly* disagree with and even object to this.

One of the biggest achievements of the KDE community is having
become multi-generational and turnover-proof, whereas most pro-
jects peter out when their initial developer generation moves
on. Note how few people in this thread were around in 1998.

I attribute this to a few traits of our structure:

* Flat hierarchies and few, mostly informally held titles.

* Broad, equal write access.

* Encouraging people to feel responsible for what we make.

These things reinforce each other in multiple ways. If main-
tainers are not entrenched positions, they're easy to replace
when they move on (whether they can accept this themselves or
not). Once you codify them in ACLs (and yes, we do this in
some places already, and I think this was a mistake as well)
you add inertia because those ACLs need to be updated, and
someone needs to work up the gumption to ask for that update.

(Case study of what can happen when we lose track of this:
We lost KDM. There are many more positive case studies where
contributors kept projects alive once maintainers disappeared
just by slowly ramping up their involvement, without needing
hand over the keys flag days.)

If maintainers aren't entrenched, you also can't rely on them
picking up the slack; hence encouraging stepping up.

How do you become a maintainer? By actively doing review,
for one. I.e. it's up to *maintainers* to stay active and
involve themselves in the process, and provide guidance so
that good code goes in and bad code stays out. The safety
net we have for review working is our brains when making
or picking through arguments.

The argument but you can still bypass Gerrit and push
directly, this is just social etiquette doesn't matter
because the whole thing is about social etiquette. The
ACLs we already have reflect our social etiquette, and
that means we need to be careful and think smart about
evolving it.

Personally I think social etiquette encouraging the use
of review tools is fine. Social etiquette that entrenches
some developers as special on those review tools is not.


Cheers,
Eike


Re: Using Gerrit for code review in KDE

2014-09-13 Thread Eike Hein



On 13.09.2014 20:21, Ivan Čukić wrote:

I agree, +2 should be retained by the maintainer, or a smaller set of
developers as decided by the maintainer.


Or perhaps it simply turns out that the whole idea of
*having* a '+2' is incompatible with the KDE community
in the first place.

Do we really want arbitrary design specifics of a tool
to dictate how we work together?


Cheers,
Eike



Re: Using Gerrit for code review in KDE

2014-09-13 Thread Eike Hein



On 13.09.2014 21:10, Kevin Krammer wrote:

So your suggestion would be to not have +2 but a policy of some sort that only
the +1 of the maintainer, if there is an active one, is considered as go
ahead?


Basically my thinking is roughly this: It actually happens
extremely rarely in practice that something gets committed
that needs to be reverted because it's actively objectiona-
ble. As Ivan pointed out, few of us will ever commit any-
thing if we're not confident it would meet with the approval
of our peers. We generally do a good job with seeking out
the opinion and approval of those in the know.

Yes, this requires a lot of trust - but we've always known
this; broad, equal write access requires just as much trust.
That's why we have a process for getting developer access
that tries to make sure those getting access have been
around long enough to understand the etiquette and can be
reasonably worked with; it's why getting developer access
involves peer review by other developers. The idea is that
trust can flow from this - when someone has a KDE developer
account you want to be able to rely on the fact that they
should have that account.

Having a +2 and restricting it to maintainers says we can't
trust KDE developer account holders. If this were actually
true, I think it would imply our process is broken on the
other end. The Manifesto is written in the spirit of rely-
ing on this trust mechanic.

But as said, I don't think it's actually true. I think we
*can* trust KDE developer account holders, and that's why
we don't need an extra safety net in the form of having a
+2 and restricting it to maintainers.

It's biasing the system in the wrong direction, and tries
to plan for an eventuality that happens extremely rarely
(and we have other safety nets: test suites, CI, beta
releases, ...), at the cost of making maintainer succession
more difficult down the road.

Maintainers should always think about the maintainers who
will one day replace them and make sure they can.



If I brainstorm about alternatives I find:

* let maintainers have -2 as pro-active variation of reverting


That still requires flagging people as maintainers in a
DB, though ...

We already flag maintainers on projects.kde.org, which as
mentioned I think was a mistake, but it mainly came about
for logistical reasons, not for security reasons. I think
we need to avoid using this as a precedent to entrench
maintainers elsewhere ACL-wise.



* build social ettiquette to always wait for the maintainer's +1 but at most
for a certain grace period, e.g. one week


I think this etiquette basically already exists in prac-
tice.

If I write a patch that touched complicated code in plasma-
framework that I know Marco knows much better than I do,
I'm not going to commit it without him having weighed in
on it. If Marco's on vacation I'll at least make sure
someone else thought through the problem and came to the
same conclusion as I did.

I fixed some bugs in KIO lately around file previews, and
struck up an in-depth conversation with David about it and
made sure I got his review and input for the changes I
needed.

And so on ... all without requiring a +2.



* have everyone get +2 and use etiquette to do that only if there is already
strong agreement or the grace period has passed


Seems best to me, assuming we can't change Gerrit to
remove the distinction entirely.



Cheers,
Kevin


Cheers,
Eike


Re: Using Gerrit for code review in KDE

2014-09-13 Thread Eike Hein



On 13.09.2014 22:50, Sven Brauch wrote:

That's my opinion as well. It would be nice to have an explicit way to
differentiate the I think this patch is okay, but I'm not very
familiar with the code you changed (+1) and I'm confident this patch
is fine (+2) cases, and I think everyone with a KDE dev account is
capable of deciding which one to select by himself when reviewing,
without a technical restriction on what one can do.


Yeah, that's something I'm OK with too. Maybe we can even
adapt the UI to use strings like Sven proposes?



Greetings!


Cheers,
Eike


Re: Using Gerrit for code review in KDE

2014-09-09 Thread Eike Hein



On 09.09.2014 15:51, Jan Kundrát wrote:

Hi,
we agreed on the Frameworks BoF that the following two repos are now
using Gerrit for some initial testing:


Exclusively, or do they remain on ReviewBoard as well?



Cheers,
Eike


Re: Using Gerrit for code review in KDE

2014-09-07 Thread Eike Hein



On 07.09.2014 11:00, Jan Kundrát wrote:

Hi folks,
as requested by Ben, I would like to accounce that Trojita
(extragear/pim/trojita) is now using Gerrit [1] for patch review.

The system is open for other KDE projects as well -- if you're
interested, see [2] for further details, or come to my talk today at
14:00 in room #2 at the Akademy to learn more. Thanks to
CESNET/FI-Ware/XIFI for providing the infrastructure to make this possible.


Exciting!

I'm curious however, what's the state of manifesto-compliance[1]
for the Gerrit instance? Does KDE Sysadmin have admin access and
the ability to get the data out if needed?

This isn't meant as an attack of any sorts; I think experimentation
is good. I'd just like to know the bases are covered.

1= The online services bit on http://manifesto.kde.org/commitments.html




With kind regards,
Jan


Cheers,
Eike


Re: Review Request 119498: Report crashes of KDE apps in Apple OS X (2)

2014-07-27 Thread Rolf Eike Beer
 In this review we have three portability problems:
 
 1. On Apple OS X, Dr Konqi's dialog box hides itself underneath the main
 window of the app that has just crashed, so is effectively useless. This
 appears to be because Dr Konqi is started by a Linux/Unix method (fork() +
 exec()?). If an app is started with the Apple OS X open command, it
 always appears on top. The patch just raises the dialog box.

Maybe that should be unconditionally done, who knows what the next fancy Linux 
DE would do.

 2. When formatting the backtrace output, Dr Konqi crashes (with an ASSERT)
 on the last line. This appears to be an error in the algorithm used (i.e.
 also a bug in Linux KDE), but the patch is treating it as an Apple OS X
 portability problem for now.

From staring at the code I suspect that this has something to do with 
additional linebreaks in the lines. From what I see the input lines may also 
contain linebreaks, this is why the internal line map has holes in the key 
sequence. I suspect the crash you see is somehow related to if the last line 
itself has \n in it or not or something like that. Looks like this needs a 
unittest ;)

Greetings,

Eike

signature.asc
Description: This is a digitally signed message part.


Review Request 119280: Add the Web Shortcuts KCM from kde-baseapps/konq to the KIO framework

2014-07-14 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119280/
---

Review request for kdelibs and David Faure.


Repository: kio


Description
---

As discussed, this adds the Web Shortcuts KCM, formerly shipped as part as 
Konqueror, to the KIO framework, where the URI Filters framework it configures 
resides as well. This makes more sense than stuffing it into workspace, since 
Web Shortcuts have many app level downstreams (e.g. Konversation, Okular and 
Konsole) which try to run the KCM via kcmshell5, and may not be running inside 
Plasma Desktop at the time.

I've lightly modified the code to make it build, and made the naming more 
consistent - webshortcuts is now used throughout where previously was a mix 
of ebrowsing and kurifilt. This does also mean the .po name changed, but 
the KCM only contains a single string.

I'm the least confident on the CMake stuff, especially the TRANSLATION_DOMAIN 
redefinition, so I'd be happy for review.


Diffs
-

  src/kcms/webshortcuts/main.cpp PRE-CREATION 
  src/kcms/webshortcuts/webshortcuts.desktop PRE-CREATION 
  src/urifilters/ikws/CMakeLists.txt 4efe24e 
  src/CMakeLists.txt 6f8373f 
  src/kcms/CMakeLists.txt PRE-CREATION 
  src/kcms/webshortcuts/CMakeLists.txt PRE-CREATION 
  src/kcms/webshortcuts/Messages.sh PRE-CREATION 
  src/kcms/webshortcuts/main.h PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119280/diff/


Testing
---


Thanks,

Eike Hein



Review Request 119130: Fix template discovery in KNewFileMenu (incorrect port to QStandardPaths)

2014-07-05 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119130/
---

Review request for kdelibs and David Faure.


Repository: kio


Description
---

KNewFileMenu was incorrectly ported to QStandardPaths, listing directories 
where it actually wants to collect template files within them. This patch fixes 
it.


Diffs
-

  src/filewidgets/knewfilemenu.cpp 4f1ca10 

Diff: https://git.reviewboard.kde.org/r/119130/diff/


Testing
---


Thanks,

Eike Hein



Re: Review Request 119130: Fix template discovery in KNewFileMenu (incorrect port to QStandardPaths)

2014-07-05 Thread Eike Hein


 On July 5, 2014, 1:11 p.m., Mark Gaiser wrote:
  src/filewidgets/knewfilemenu.cpp, line 874
  https://git.reviewboard.kde.org/r/119130/diff/1/?file=287354#file287354line874
 
  There is no need for this check since QStandardPaths::locateAll only 
  returns paths that exist: 
  https://qt.gitorious.org/qt/qtbase/source/76371c4d560c5a13f54f1ed9078fb68a393aa1bd:src/corelib/io/qstandardpaths.cpp#L380
  
  Or that's how i read the code.

Yeah, this can be dropped - it's habitual sanity-checking paranoia.


 On July 5, 2014, 1:11 p.m., Mark Gaiser wrote:
  src/filewidgets/knewfilemenu.cpp, lines 875-878
  https://git.reviewboard.kde.org/r/119130/diff/1/?file=287354#file287354line875
 
  You should be able to replace this with:
  files.append(dir.entryList(QStringList()  *.desktop, QDir::Files));
  
  Yes, append also takes a QListT container type.  If that doesn't work 
  then you also have the option of doing:
  files += dir.entryList(QStringList()  *.desktop, QDir::Files);
  
  Cool huh, one liners? :)

entryList() doesn't return full absolute paths, only file names, so your one 
liner isn't actually functionally equivalent (and full paths are needed there).


- Eike


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119130/#review61646
---


On July 5, 2014, 12:50 p.m., Eike Hein wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119130/
 ---
 
 (Updated July 5, 2014, 12:50 p.m.)
 
 
 Review request for kdelibs and David Faure.
 
 
 Repository: kio
 
 
 Description
 ---
 
 KNewFileMenu was incorrectly ported to QStandardPaths, listing directories 
 where it actually wants to collect template files within them. This patch 
 fixes it.
 
 
 Diffs
 -
 
   src/filewidgets/knewfilemenu.cpp 4f1ca10 
 
 Diff: https://git.reviewboard.kde.org/r/119130/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Eike Hein
 




Re: Review Request 119130: Fix template discovery in KNewFileMenu (incorrect port to QStandardPaths)

2014-07-05 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119130/
---

(Updated July 5, 2014, 1:26 p.m.)


Review request for kdelibs and David Faure.


Changes
---

Drop redundant QDir::exists() check.


Repository: kio


Description
---

KNewFileMenu was incorrectly ported to QStandardPaths, listing directories 
where it actually wants to collect template files within them. This patch fixes 
it.


Diffs (updated)
-

  src/filewidgets/knewfilemenu.cpp 4f1ca10 

Diff: https://git.reviewboard.kde.org/r/119130/diff/


Testing
---


Thanks,

Eike Hein



Re: Review Request 119130: Fix template discovery in KNewFileMenu (incorrect port to QStandardPaths)

2014-07-05 Thread Eike Hein


 On July 5, 2014, 1:51 p.m., Mark Gaiser wrote:
  Last question, could you make a unittest for this?
  
  I'm not quite sure if it's even possible since this is in a private class. 
  You might be able to test this using KNewFileMenu::checkUpToDate?
  KIO has very few unittests for KNewFileMenu, they can be found in 
  knewfilemenutest.cpp. More is welcome there.

No clue to be honest since the operation relies on template files being 
installed, which aren't in the same framework (the templates are part of the 
libkonq codebase in kde-baseapps ...). I'm not experienced with writing 
KIO-related unit tests so I'd likely need help there, and I'd strongly advice 
against waiting for it right now (my todo is large and frightning).


- Eike


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119130/#review61649
---


On July 5, 2014, 1:26 p.m., Eike Hein wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119130/
 ---
 
 (Updated July 5, 2014, 1:26 p.m.)
 
 
 Review request for kdelibs and David Faure.
 
 
 Repository: kio
 
 
 Description
 ---
 
 KNewFileMenu was incorrectly ported to QStandardPaths, listing directories 
 where it actually wants to collect template files within them. This patch 
 fixes it.
 
 
 Diffs
 -
 
   src/filewidgets/knewfilemenu.cpp 4f1ca10 
 
 Diff: https://git.reviewboard.kde.org/r/119130/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Eike Hein
 




Re: Review Request 119130: Fix template discovery in KNewFileMenu (incorrect port to QStandardPaths)

2014-07-05 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119130/
---

(Updated July 5, 2014, 6:54 p.m.)


Status
--

This change has been marked as submitted.


Review request for kdelibs and David Faure.


Repository: kio


Description
---

KNewFileMenu was incorrectly ported to QStandardPaths, listing directories 
where it actually wants to collect template files within them. This patch fixes 
it.


Diffs
-

  src/filewidgets/knewfilemenu.cpp 4f1ca10 

Diff: https://git.reviewboard.kde.org/r/119130/diff/


Testing
---


Thanks,

Eike Hein



Re: Review Request 118816: Don't crash konsole when a container destructs

2014-06-18 Thread Eike Hein
/qeventdispatcher_glib.cpp:426
#61 0x7f04f849c1ba in QPAEventDispatcherGlib::processEvents 
(this=0x2100e50, flags=...) at eventdispatchers/qeventdispatcher_glib.cpp:123
#62 0x7f0501092706 in QEventLoop::processEvents (this=0x7e4d8b60, 
flags=...) at kernel/qeventloop.cpp:136
#63 0x7f05010929e9 in QEventLoop::exec (this=0x7e4d8b60, flags=...) at 
kernel/qeventloop.cpp:212
#64 0x7f050109613b in QCoreApplication::exec () at 
kernel/qcoreapplication.cpp:1188
#65 0x7f0501674496 in QGuiApplication::exec () at 
kernel/qguiapplication.cpp:1446
#66 0x7f0501ef977d in QApplication::exec () at kernel/qapplication.cpp:2767
#67 0x7f05082f6aaf in kdemain (argc=1, argv=0x7e4d8ed8) at 
/home/sho/devel/src/kde/applications/konsole/src/main.cpp:92
#68 0x00400bf2 in main (argc=1, argv=0x7e4d8ed8) at 
/home/sho/devel/build/kde/applications/konsole/src/konsole_dummy.cpp:3


- Eike Hein


On June 18, 2014, 5:50 p.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118816/
 ---
 
 (Updated June 18, 2014, 5:50 p.m.)
 
 
 Review request for KDE Base Apps and Eike Hein.
 
 
 Repository: konsole
 
 
 Description
 ---
 
 When containers are destructed, they emit a signal that the splitter is 
 connected to which removes and unregisters the container from the splitter.
 A crash happens when the splitter is destroyed before the container, so a 
 slot in a deleted splitter is called, tries to unregister the container, and 
 segfaults.
 I added a destructor to ViewSplitter which unregisters all it's containers, 
 this fixes the crash here on closing of a tab in yakuake and on closing a tab 
 in konsole.
 
 
 Diffs
 -
 
   src/ViewSplitter.h c1e4552 
   src/ViewSplitter.cpp bfc727e 
 
 Diff: https://git.reviewboard.kde.org/r/118816/diff/
 
 
 Testing
 ---
 
 Manual testing, it seems to work fine.
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Review Request 110328: Add config option to silently create initial password-less wallet

2014-06-11 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/110328/
---

(Updated June 11, 2014, 3:35 p.m.)


Status
--

This change has been discarded.


Review request for KDE Runtime and Harald Sitter.


Repository: kde-runtime


Description
---

This patch adds a UI-less config option to kwalletd that makes it create the 
initial local wallet silently with an empty password instead of prompting the 
user to enter one.

It's a change desired by downstream consumers Kubuntu and Netrunner, and 
perhaps others, and recreates a modification they used to carry for KDE 3. 
Their goal is to make KWallet mostly invisible to the user during routine 
operations, but still have users benefit from encrypted password storage behind 
the scenes.

As such the config option is intended to be set by distributions. The new 
behavior is disabled by default.

In the interest of keeping the delta between upstream and downstream as small 
as possible I'd say it makes sense to pick this up.


Diffs
-

  kwalletd/kwalletd.h e8e74c3 
  kwalletd/kwalletd.cpp fa9fc11 

Diff: https://git.reviewboard.kde.org/r/110328/diff/


Testing
---

Test package for Kubuntu by Harald Sitter, operation verified at runtime.


Thanks,

Eike Hein



Re: Review Request 116097: No longer use strlcpy in kstartupconfig

2014-02-26 Thread Rolf Eike Beer

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116097/#review50982
---


When you start poking at this code, why not kill the entire fopen/fgets/fixed 
size buffer stuff and replace it with std streams or something like that?

- Rolf Eike Beer


On Feb. 26, 2014, 6:11 p.m., Alexander Richardson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116097/
 ---
 
 (Updated Feb. 26, 2014, 6:11 p.m.)
 
 
 Review request for kde-workspace.
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 No longer use strlcpy in kstartupconfig
 
 This means we no longer need to find libbsd on Linux.
 kstartupconfig is now uses std::string instead of pure C strings, but
 the performance difference should be minimal (and it is less error-prone).
 
 
 Diffs
 -
 
   CMakeLists.txt bce3cd1de65b8420a859c342db981919965071ba 
   cmake/modules/FindLibBSD.cmake 6383083023cdbfbeffcab0cef98f48102e593d38 
   kstartupconfig/CMakeLists.txt ceebd6a65af4cdaa717cfb0dcff5097121cf48d9 
   kstartupconfig/kstartupconfig.c d8e65f48a75dad49fe6c585f89260c2a6d483809 
 
 Diff: https://git.reviewboard.kde.org/r/116097/diff/
 
 
 Testing
 ---
 
 compiles
 
 
 Thanks,
 
 Alexander Richardson
 




Re: Review Request 115689: Fix khtml/ecma/xmlhttprequest.cpp to properly handle HEAD requests

2014-02-12 Thread Rolf Eike Beer

Am 12.02.2014 10:56, schrieb Dawit Alemayehu:

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115689/
---

(Updated Feb. 12, 2014, 9:56 a.m.)


Review request for kdelibs and Andrea Iacovitti.


Changes
---

Uploaded the patch.


It looks to me as if the patch also contains the stuff for RR 115651.

Eike


Re: Review Request 115651: Fix HTTP redirection handling (3XX status code)

2014-02-12 Thread Rolf Eike Beer
Am Mittwoch, 12. Februar 2014, 17:10:49 schrieb Andrea Iacovitti:
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/115651/#review49669
 ---
 
 
 As stated in the bug report it is also true that every other browsers
 rewrite POST method to GET when following 301/302 redirections. This
 behavior could also be verified in curl by issuing the following commands:
 curl -L --data fakepostdata
 http://greenbytes.de/tech/tc/httpredirects/redirect_with_status.cgi?301
 curl -L --data fakepostdata
 http://greenbytes.de/tech/tc/httpredirects/redirect_with_status.cgi?302 We
 could/should do the same for compatibility.
 In that case the snippet of code that handles 301-303 http status codes may
 assume this form:
 
 } else if (m_request.responseCode = 301  m_request.responseCode=
 303) { // NOTE: This is wrong according to RFC 2616 (section 10.3.[2-4,8]).
 // However, because almost all client implementations treat a 301/302 //
 response as a 303 response in violation of the spec, many servers // have
 simply adapted to this way of doing things! Thus, we are // forced to do
 the same thing. Otherwise, we loose compatibility and // might not be able
 to correctly retrieve sites that redirect. if (m_request.responseCode ==
 301) { // Moved permanently
 setMetaData(QLatin1String(permanent-redirect), QLatin1String(true)); if
 (m_request.method == HTTP_POST) {
 m_request.method = HTTP_GET; // FORCE a GET
 setMetaData(QLatin1String(redirect-to-get),
 QLatin1String(true)); }
 } else if (m_request.responseCode == 302) { // Moved temporarily
 if (m_request.method == HTTP_POST) {
 m_request.method = HTTP_GET; // FORCE a GET
 setMetaData(QLatin1String(redirect-to-get),
 QLatin1String(true)); }
 } else { // 303 See Other
 if (m_request.method != HTTP_HEAD) {
 m_request.method = HTTP_GET; // FORCE a GET
 setMetaData(QLatin1String(redirect-to-get),
 QLatin1String(true)); }
 }
 }
 
 ...or something like that.

switch () { … }

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Moving Baloo forward

2014-01-28 Thread Rolf Eike Beer
Am Dienstag, 28. Januar 2014, 20:10:13 schrieb Ivan Čukić:
  To move a file to another machine and have the metadata be copied and re-
  indexed on that new machine as well. The copy process just needs to take
  care of transfering the xattr. This can even work when using a USB stick
  or so as interim medium.
 
 I'm all for xattrs, but this thread really raises a nice question of which
 annotations/tags/whatever should be public and which should not.

IMHO the default should be privacy first. Probably everyone of us has laughed 
about someone who accidentially published either metadata or deleted text 
(remember MS Word document history or something)? I'm absolutely fine if you 
want this that you do this. But most people will not be aware of it, even less 
of the implications. So it should be deactivated by default and only activated 
explicitely.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Moving Baloo forward

2014-01-27 Thread Rolf Eike Beer
Am Freitag, 24. Januar 2014, 15:59:12 schrieb Vadim Zhukov:
 2014/1/24 Sebastian Kügler se...@kde.org:
  On Friday, January 24, 2014 01:24:54 Vadim Zhukov wrote:
  in the best case you'll have two totally different codepaths
  that you'll have to manage.
  
  This should be worst case, I think. In the best case, there's xattr
  support, meaning another code path isn't needed.
 
 It's doubtful at least whether xattr support is a good thing, because
 it's too easily gets misused: for example, there were viruses on
 Windows which effectively hidden themselves in NTFS streams. And there
 are personal data leaking issues I've pointed out earlier. But the
 question xarttrs: to be or not to be everywhere is totally offtopic,
 because it's what OS developers should decide.

I am against storing metadata in xattrs, but more for personal opinion than 
for anything I can express in technical terms. What could be done and which 
uses xattrs for what I seem them for is to just store a unique identifier in 
the xattrs (e.g. a GUID) which makes it easier to map the file to its metadata 
in the database, maybe together with some sort of checksum to detect 
modifications. That way no accidential information leak will happen if that 
thing is copied elsewhere.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

2014-01-04 Thread Rolf Eike Beer

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114737/#review46765
---



kinfocenter/Modules/opengl/opengl.cpp
https://git.reviewboard.kde.org/r/114737/#comment33389

This allocates a new object on the heap that is never freed. Just use 
QProcess pipe;


- Rolf Eike Beer


On Jan. 4, 2014, 9 a.m., Wolfgang Bauer wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114737/
 ---
 
 (Updated Jan. 4, 2014, 9 a.m.)
 
 
 Review request for kde-workspace and David Stephen Hubner.
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 This patch reimplements the ReadPipe() function by using QProcess instead of 
 popen().
 This should make it more portable.
 
 As a positive side-effect, this also removes those sh: lspci: command not 
 found. messages when run in Konsole and lspci is not in the user's path.
 
 This was suggested on the kde-core-devel mailinglist in November:
 http://lists.kde.org/?l=kde-core-develm=138407113011843w=2
 http://lists.kde.org/?l=kde-core-develm=138409755820003w=2
 
 
 Diffs
 -
 
   kinfocenter/Modules/opengl/opengl.cpp 8901957 
 
 Diff: https://git.reviewboard.kde.org/r/114737/diff/
 
 
 Testing
 ---
 
 Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
 (not in the user's path). The OpenGL module showed the 3D accelerator info 
 correctly in both cases.
 With lspci removed completely it showed unknown as expected.
 
 
 Thanks,
 
 Wolfgang Bauer
 




Re: Review Request 114737: KInfocenter/OpenGL: reimplement the ReadPipe() function with QProcess

2014-01-04 Thread Rolf Eike Beer

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114737/#review46798
---


Looks fine to me, although I have not tested it.

One minor nitpick (that doesn't deserve a new diff): you could make the 
FileName argument to that function const QString .

- Rolf Eike Beer


On Jan. 4, 2014, 4:54 p.m., Wolfgang Bauer wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/114737/
 ---
 
 (Updated Jan. 4, 2014, 4:54 p.m.)
 
 
 Review request for kde-workspace and David Stephen Hubner.
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 This patch reimplements the ReadPipe() function by using QProcess instead of 
 popen().
 This should make it more portable.
 
 As a positive side-effect, this also removes those sh: lspci: command not 
 found. messages when run in Konsole and lspci is not in the user's path.
 
 This was suggested on the kde-core-devel mailinglist in November:
 http://lists.kde.org/?l=kde-core-develm=138407113011843w=2
 http://lists.kde.org/?l=kde-core-develm=138409755820003w=2
 
 
 Diffs
 -
 
   kinfocenter/Modules/opengl/opengl.cpp 8901957 
 
 Diff: https://git.reviewboard.kde.org/r/114737/diff/
 
 
 Testing
 ---
 
 Ran KInfocenter with lspci in /usr/bin/ (i.e. in the user's path) and /sbin/ 
 (not in the user's path). The OpenGL module showed the 3D accelerator info 
 correctly in both cases.
 With lspci removed completely it showed unknown as expected.
 
 
 Thanks,
 
 Wolfgang Bauer
 




Re: Review Request 113779: KInfocenter/OpenGL: fix ReadPipe() in the case that the command cannot be run

2013-11-10 Thread Rolf Eike Beer
 ReadPipe() doesn't return 0 as expected in the case that the command is not
 found. but the length of sh's output which is command not found in this
 case. This is because popen() does not fail if the command is not found,
 because it _can_ run sh. (according to the man page, popen calls /bin/sh
 -c command) To fix this, ReadPipe() should check the return code of the
 call to pclose() (see man pclose), and return 0 if this is not equal to
 0.

Can't this be ported to simply use QProcess instead?

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 113447: Attempt to unbreak pre-configuring QML applets via desktop scripting, take 2

2013-10-29 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113447/
---

(Updated Oct. 29, 2013, 11:13 a.m.)


Status
--

This change has been marked as submitted.


Review request for kdelibs and Plasma.


Repository: kdelibs


Description
---

Please see https://git.reviewboard.kde.org/r/113443/ for the earlier attempt 
and Aaron's review, which lead to this new take.


Diffs
-

  plasma/configloader.cpp 6e97cb9 

Diff: http://git.reviewboard.kde.org/r/113447/diff/


Testing
---


Thanks,

Eike Hein



Re: Review Request 113447: Attempt to unbreak pre-configuring QML applets via desktop scripting, take 2

2013-10-27 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113447/
---

(Updated Oct. 27, 2013, 12:19 p.m.)


Review request for kdelibs, Plasma, Aaron J. Seigo, and Marco Martin.


Repository: kdelibs


Description
---

Please see https://git.reviewboard.kde.org/r/113443/ for the earlier attempt 
and Aaron's review, which lead to this new take.


Diffs
-

  plasma/configloader.cpp 6e97cb9 

Diff: http://git.reviewboard.kde.org/r/113447/diff/


Testing
---


Thanks,

Eike Hein



Re: Review Request 113447: Attempt to unbreak pre-configuring QML applets via desktop scripting, take 2

2013-10-27 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113447/
---

(Updated Oct. 27, 2013, 12:19 p.m.)


Review request for kdelibs and Plasma.


Repository: kdelibs


Description
---

Please see https://git.reviewboard.kde.org/r/113443/ for the earlier attempt 
and Aaron's review, which lead to this new take.


Diffs
-

  plasma/configloader.cpp 6e97cb9 

Diff: http://git.reviewboard.kde.org/r/113447/diff/


Testing
---


Thanks,

Eike Hein



Re: Review Request 112528: Make it possible to preserve mtime for copy jobs not just move ones

2013-09-05 Thread Rolf Eike Beer
 Diffs
 -
 
   kio/kio/job.cpp 8ff088c
 
 Diff: http://git.reviewboard.kde.org/r/112528/diff/

Is it reviewboard fooling me or is there no diff?

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 112528: Make it possible to preserve mtime for copy jobs not just move ones

2013-09-05 Thread Rolf Eike Beer
Am Donnerstag 05 September 2013, 08:43:48 schrieb Rolf Eike Beer:
  Diffs
  -
  
kio/kio/job.cpp 8ff088c
  
  Diff: http://git.reviewboard.kde.org/r/112528/diff/
 
 Is it reviewboard fooling me or is there no diff?

This one does exist, I sent the mail for the wrong RR:

http://git.reviewboard.kde.org/r/112529/diff/

This does not exist.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 112241: Fix Show Launcher when not running option in taskbar widget

2013-08-25 Thread Eike Hein


 On Aug. 24, 2013, 3:17 p.m., Eike Hein wrote:
  Hm, on the face of it, this patch doesn't really make sense ... launcher 
  items don't have an associated task, so the function should already return 
  early and the extra condition should be redundant. Unless there's a race 
  condition in the library somewhere ... but then it still wouldn't crash on 
  translating a QPoint.
  
  Thank you for the patch, but I really still have to find a way to reproduce 
  this bug before just applying this blindly - it might be treating a symptom 
  instead of addressing the root cause.
 
 Bhushan Shah wrote:
 Are you sure that this happens on 32 bit only?
 
 Eike Hein wrote:
 No, I'm not - except I can't reproduce it on any of my 64 bit machines, 
 and I have to assume if it were a widespread bug, the number of reports we'd 
 be getting would be much, much higher. Note that we didn't even get any 
 reports through any of the pre-releases about this, AFAIR.
 
 I'm typing this from a cellphone right now, but I hope when I get home 
 tonight I'll finally get around to setting up a 32 bit VM, and I'm hoping 
 it'll crash there so I can throw all the gdb/valgrind/asan at it we got :).
 
 Thomas Lübking wrote:
 What bug and is WId involved?
 
 Bhushan Shah wrote:
 https://bugs.kde.org/show_bug.cgi?id=322283
 
 Hrvoje Senjan wrote:
 @Thomas
 bug#322283
 
 Thomas Lübking wrote:
 Thanks.
 
 From a brief review of libtaskmanager, TaskGroup::getMemberById(int id) 
 returns AbstractGroupableItem* which could be (reimplemented)
 - TaskGroup*
 - LauncherItem*
 - TaskItem*
 but is happily static_cast'ed to TaskItem* and then accessed.
 
 The fact(?) that itemType returns LauncherItemType indicates that there 
 can very well be different returns and then you're accessing random memory 
 portions - it does absolutely not matter that the function pointer for 
 ::task() points into garbage when the item is actually a Launcher - the 
 garbage is still rather not null, there's no guarantee that this basic deref 
 somehow would crash or fail (virtual ::task() needed to be moved to 
 AbstractGroupableItem then)
 
 To know whether or why this has different implications on different 
 architectures:
 Valgrind will tell you.
 
 For the moment
 - either ::getMemberById() is supposed to return different types than 
 TaskItem here, then static_cast needs to be qobject_cast or dynamic_cast 
 (pending on linkage)
 - or and ::getMemberById() that is *not* TaskItem must be considered a 
 bug, then i'd start by adding an 
 Q_ASSERT(!static_castTaskManager::AbstractGroupableItem*(taskItem-itemType()
  == TaskManager::TaskItemType))
 
 
 
 I don't really know, but as this thing seems to manage all kinds of 
 items, but only updating rects for TaskItem's (and actually *every* grouped 
 TaskItem for a TaskGroupType return ... so we don't get more bugs on windows 
 don't minimize to proper icon ;-P) makes sense, the correct solution is 
 probably indeed:
 
 AbstractGroupableItem *item = rootGroup()-getMemberById(id);
 
 if (item-itemType() == TaskManager::LauncherItemType)
return; // launchers have no windows, we just cause X11 errors and 
 with a little luck a stupid gobject abort
 if (item-itemType() == TaskManager::TaskGroupType) {
// build a WId list of all grouped Items
...
 } else //if (item-itemType() == TaskManager::TaskItemType) {
tasks  static_castTaskManager::TaskItem*(taskItem)-task(); 
 }
 
 // search parrent and other juggling to figure the proper rectangle
 ...

Thomas: Building the list of group child windows is already done (the traversal 
happens on the QML side).


- Eike


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112241/#review38481
---


On Aug. 24, 2013, 2 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112241/
 ---
 
 (Updated Aug. 24, 2013, 2 p.m.)
 
 
 Review request for kde-workspace, Plasma and Eike Hein.
 
 
 Description
 ---
 
 Fix the crash in plasma-desktop caused by newer QML taskbar widget.
 
 Simple steps to reproduce this crash.
 
 1) Pin any task/application to taskbar using show launcher when not running 
 option.
 2) Close application.
 3) Desktop crashes.
 
 Reason :
 
 1) In Tasks::itemGeometryChanged(int,int,int,int,int) line 300 it checks for 
 three conditions, 
 
   - pointer to task is not null
   - taskItem itself is not null
   - scene is not null
 
 2) This condition gets false when item is LauncherItem. In function later 
 line 334 when calling iconRect.moveTopLeft

Re: Review Request 112241: Fix Show Launcher when not running option in taskbar widget

2013-08-25 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112241/
---

(Updated Aug. 25, 2013, 2:44 p.m.)


Status
--

This change has been discarded.


Review request for kde-workspace, Plasma and Eike Hein.


Description
---

Fix the crash in plasma-desktop caused by newer QML taskbar widget.

Simple steps to reproduce this crash.

1) Pin any task/application to taskbar using show launcher when not running 
option.
2) Close application.
3) Desktop crashes.

Reason :

1) In Tasks::itemGeometryChanged(int,int,int,int,int) line 300 it checks for 
three conditions, 

  - pointer to task is not null
  - taskItem itself is not null
  - scene is not null

2) This condition gets false when item is LauncherItem. In function later line 
334 when calling iconRect.moveTopLeft(QPoint) function it gets crashed.

Patch :

This patch adds check in if condition to check if taskItem is 
TaskManager::LauncherItemType and return from function if this is launcher item.


Diffs
-

  plasma/desktop/applets/tasks/tasks.cpp c4aef4b 

Diff: http://git.reviewboard.kde.org/r/112241/diff/


Testing
---

Testing

compilation - check
installation - check
plasmoidviewer - check
in panel - check
independently - check


Thanks,

Bhushan Shah



Re: Review Request 112241: Fix Show Launcher when not running option in taskbar widget

2013-08-24 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112241/#review38481
---


Hm, on the face of it, this patch doesn't really make sense ... launcher items 
don't have an associated task, so the function should already return early and 
the extra condition should be redundant. Unless there's a race condition in the 
library somewhere ... but then it still wouldn't crash on translating a QPoint.

Thank you for the patch, but I really still have to find a way to reproduce 
this bug before just applying this blindly - it might be treating a symptom 
instead of addressing the root cause.

- Eike Hein


On Aug. 24, 2013, 2 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112241/
 ---
 
 (Updated Aug. 24, 2013, 2 p.m.)
 
 
 Review request for kde-workspace, Plasma and Eike Hein.
 
 
 Description
 ---
 
 Fix the crash in plasma-desktop caused by newer QML taskbar widget.
 
 Simple steps to reproduce this crash.
 
 1) Pin any task/application to taskbar using show launcher when not running 
 option.
 2) Close application.
 3) Desktop crashes.
 
 Reason :
 
 1) In Tasks::itemGeometryChanged(int,int,int,int,int) line 300 it checks for 
 three conditions, 
 
   - pointer to task is not null
   - taskItem itself is not null
   - scene is not null
 
 2) This condition gets false when item is LauncherItem. In function later 
 line 334 when calling iconRect.moveTopLeft(QPoint) function it gets crashed.
 
 Patch :
 
 This patch adds check in if condition to check if taskItem is 
 TaskManager::LauncherItemType and return from function if this is launcher 
 item.
 
 
 Diffs
 -
 
   plasma/desktop/applets/tasks/tasks.cpp c4aef4b 
 
 Diff: http://git.reviewboard.kde.org/r/112241/diff/
 
 
 Testing
 ---
 
 Testing
 
 compilation - check
 installation - check
 plasmoidviewer - check
 in panel - check
 independently - check
 
 
 Thanks,
 
 Bhushan Shah
 




Re: Review Request 112241: Fix Show Launcher when not running option in taskbar widget

2013-08-24 Thread Eike Hein


 On Aug. 24, 2013, 3:17 p.m., Eike Hein wrote:
  Hm, on the face of it, this patch doesn't really make sense ... launcher 
  items don't have an associated task, so the function should already return 
  early and the extra condition should be redundant. Unless there's a race 
  condition in the library somewhere ... but then it still wouldn't crash on 
  translating a QPoint.
  
  Thank you for the patch, but I really still have to find a way to reproduce 
  this bug before just applying this blindly - it might be treating a symptom 
  instead of addressing the root cause.
 
 Bhushan Shah wrote:
 Are you sure that this happens on 32 bit only?

No, I'm not - except I can't reproduce it on any of my 64 bit machines, and I 
have to assume if it were a widespread bug, the number of reports we'd be 
getting would be much, much higher. Note that we didn't even get any reports 
through any of the pre-releases about this, AFAIR.

I'm typing this from a cellphone right now, but I hope when I get home tonight 
I'll finally get around to setting up a 32 bit VM, and I'm hoping it'll crash 
there so I can throw all the gdb/valgrind/asan at it we got :).


- Eike


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112241/#review38481
---


On Aug. 24, 2013, 2 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112241/
 ---
 
 (Updated Aug. 24, 2013, 2 p.m.)
 
 
 Review request for kde-workspace, Plasma and Eike Hein.
 
 
 Description
 ---
 
 Fix the crash in plasma-desktop caused by newer QML taskbar widget.
 
 Simple steps to reproduce this crash.
 
 1) Pin any task/application to taskbar using show launcher when not running 
 option.
 2) Close application.
 3) Desktop crashes.
 
 Reason :
 
 1) In Tasks::itemGeometryChanged(int,int,int,int,int) line 300 it checks for 
 three conditions, 
 
   - pointer to task is not null
   - taskItem itself is not null
   - scene is not null
 
 2) This condition gets false when item is LauncherItem. In function later 
 line 334 when calling iconRect.moveTopLeft(QPoint) function it gets crashed.
 
 Patch :
 
 This patch adds check in if condition to check if taskItem is 
 TaskManager::LauncherItemType and return from function if this is launcher 
 item.
 
 
 Diffs
 -
 
   plasma/desktop/applets/tasks/tasks.cpp c4aef4b 
 
 Diff: http://git.reviewboard.kde.org/r/112241/diff/
 
 
 Testing
 ---
 
 Testing
 
 compilation - check
 installation - check
 plasmoidviewer - check
 in panel - check
 independently - check
 
 
 Thanks,
 
 Bhushan Shah
 




Re: KDirWatch bug and the analysis. Help is welcome!

2013-08-01 Thread Rolf Eike Beer
David Faure wrote:
 On Thursday 01 August 2013 01:30:08 Mark wrote:
  However, we have been given the power of inotify which gives more detailed
  signals and lets us know which files have been created/added/modified
  which
  we should be used imho.
 
 OK. First let's imagine that it's not a hidden file. Say you create foo
 file (from the command line) in a directory currently shown in dolphin.
 When using inotify, we could get a foo was created signal, but then what?
 KDirLister is going to need more details anyway (size, mimetype, date,
 icon, etc.). To get that, it re-lists the directory. Don't say it could
 just KIO::stat the new file, it becomes very slow if many files get
 created/modified, and it creates much more complex code paths in kdirlister
 which is already complex (it would also need to handle deletion separately).
 Instead we have a single reaction to something changed in this directory
 : re-list it and update it to show the changes (after basically diff'ing
 the new listing and the old listing).

Handling delete should be much simpler than adds as you do not need to lookup 
any new information, so avoiding the whole directory scan on delete sounds 
like a good idea to me in any case, no?

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Supported Compilers / C++11 Support in KF5

2013-07-22 Thread Rolf Eike Beer
Am Montag, 22. Juli 2013, 19:14:10 schrieb Alexander Neundorf:
 On Sunday 21 July 2013, Rolf Eike Beer wrote:

  I totally agree with that. As I said: detection of this is trivial at
  CMake time, maybe I get my C++ feature detection package ready even to be
  included in 2.8.12, and if not the stuff can easily be extracted. And
 
 Yes, that would be great :-)
 You still wanted to have a rweview, right ?

Sure. ;)

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Supported Compilers / C++11 Support in KF5

2013-07-21 Thread Rolf Eike Beer
Volker Krause wrote:

 - GCC = 4.5

 - override

Explicit virtual overrides require g++ 4.7:

http://gcc.gnu.org/projects/cxx0x.html

This is trivially to work around by a CMake time check and then just define 
override to empty.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Supported Compilers / C++11 Support in KF5

2013-07-21 Thread Rolf Eike Beer
Am Sonntag, 21. Juli 2013, 14:11:07 schrieb Volker Krause:
 On Sunday 21 July 2013 13:52:06 Rolf Eike Beer wrote:
  Volker Krause wrote:
   - GCC = 4.5
   
   - override
  
  Explicit virtual overrides require g++ 4.7:
  
  http://gcc.gnu.org/projects/cxx0x.html
 
 you are right, it's also what all other sources referenced in
 http://article.gmane.org/gmane.comp.kde.devel.frameworks/3652 say, no idea
 how that slipped in then, sorry.
 
  This is trivially to work around by a CMake time check and then just
  define
  override to empty.
 
 sure, for KF5 this is not really a problem since Qt provides corresponding
 macros already. I was hoping for real unconditional usage though, since
 Akonadi will need to stay backward-compatible with Qt4 for a while longer,
 and I wanted to avoid to have to basically copy qcompilerdetection.h for
 that. Not to mention that override looks much nicer than
 Q_DECL_OVERRIDE ;)

I totally agree with that. As I said: detection of this is trivial at CMake 
time, maybe I get my C++ feature detection package ready even to be included 
in 2.8.12, and if not the stuff can easily be extracted. And afterwards you 
just add -Doverride= to CMAKE_CXX_FLAGS and everything is fine.

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 110687: DrKonqi should check for disabled version as the very first step in the reporting assistant.

2013-05-28 Thread Rolf Eike Beer

Martin Gräßlin wrote:


Could you please get some feedback from packagers. I'm not sure
whether they like words like unmaintained and upgrade. The fact
that we as upstream don't accept bugs doesn't mean it's unmaintained
by the distro and it's not said that one could upgrade (think of
Debian Stable).


I suggest something like try out a more recent version, maybe the bug 
has already been fixed.


Eike


Re: KDE Workspace broken due to upstream CMake changes

2013-05-27 Thread Rolf Eike Beer

Am 27.05.2013 09:13, schrieb Ben Cooksley:

Hi all,

It seems that a recent upstream change in CMake has now broken the
build of KDE Workspace. Can someone please fix or prod CMake upstream
into revising their policies?

The lack of warning here concerning the change is a little irritating.

-- Looking for XkbLockModifiers in X11
CMake Error at CMakeLists.txt:10 (ADD_EXECUTABLE):
  Target cmTryCompileExec70252 links to item 
/usr/lib64/libXpm.so 
  which has leading or trailing whitespace.  This is now an error 
according

  to policy CMP0004.


CMake Error: Internal CMake error, TryCompile generation of cmake 
failed

-- Looking for XkbLockModifiers in X11 - not found


That's what the policies are for at all ;)

cmake --help-policy CMP0004

So, fix whatever is causing this, and in the meantime use:

cmake_policy(SET CMP0004 OLD)

Eike


Re: Review Request 110633: Don't report crashes if version is disabled in Bugzilla

2013-05-25 Thread Rolf Eike Beer
Am Samstag 25 Mai 2013, 10:19:37 schrieb Jekyll Wu:

 Well, DrKonqi uses Product.get API to fetch product information from
 bugs.kde.org, including all available versions (either active or disabled).
 But that is not the point of those escaped reports which shouldn't be
 accepted. Even if DrKonqi doesn't know any version information on
 bugs.kde.org, when it sends a request against some disabled version,
 bugs.kde.org (since upgrading to bugzilla 4.2.5) should detect and reject
 the request, then send back a error message to drkonqi, like the screenshot
 in https://bugs.kde.org/attachment.cgi?id=78600action=edit.
 
 Now this expected rejecting behavior works for versions like 4.9.0 or
 4.10.60 (no whitespace), but not for 4.8.5 (4.8.5) (containing
 whitespace). It might be that DrKonqi sends version containing whitespace
 in a wrong way, or that bugzilla deals with version containing white space
 in a wrong way. But since Drknonqi also sends distribution information
 (like Ubuntu Packages) containing whitespace in the same way and bugzilla
 deals with that correctly, I'm inclined to think something goes wrong in
 bugzilla when dealing with versions.

Maybe it's not the space, but (). Maybe it is used as regex or something?

Eike

signature.asc
Description: This is a digitally signed message part.


Re: Review Request 110328: Add config option to silently create initial password-less wallet

2013-05-17 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110328/#review32717
---


If there are no objections I intend to commit this patch early next week.

- Eike Hein


On May 6, 2013, 5:25 p.m., Eike Hein wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/110328/
 ---
 
 (Updated May 6, 2013, 5:25 p.m.)
 
 
 Review request for KDE Runtime and Harald Sitter.
 
 
 Description
 ---
 
 This patch adds a UI-less config option to kwalletd that makes it create the 
 initial local wallet silently with an empty password instead of prompting the 
 user to enter one.
 
 It's a change desired by downstream consumers Kubuntu and Netrunner, and 
 perhaps others, and recreates a modification they used to carry for KDE 3. 
 Their goal is to make KWallet mostly invisible to the user during routine 
 operations, but still have users benefit from encrypted password storage 
 behind the scenes.
 
 As such the config option is intended to be set by distributions. The new 
 behavior is disabled by default.
 
 In the interest of keeping the delta between upstream and downstream as small 
 as possible I'd say it makes sense to pick this up.
 
 
 Diffs
 -
 
   kwalletd/kwalletd.h e8e74c3 
   kwalletd/kwalletd.cpp fa9fc11 
 
 Diff: http://git.reviewboard.kde.org/r/110328/diff/
 
 
 Testing
 ---
 
 Test package for Kubuntu by Harald Sitter, operation verified at runtime.
 
 
 Thanks,
 
 Eike Hein
 




Re: Review Request 110330: Make Prompt on access kwalletd setting apply in more situations

2013-05-17 Thread Eike Hein

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110330/#review32718
---


If there are no objections I intend to commit this patch early next week.

- Eike Hein


On May 8, 2013, 11:12 p.m., Eike Hein wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/110330/
 ---
 
 (Updated May 8, 2013, 11:12 p.m.)
 
 
 Review request for KDE Runtime and Harald Sitter.
 
 
 Description
 ---
 
 kwalletd has a Prompt when an application accesses an open wallet config 
 option. If this option is enabled (it is by default) any such access attempt 
 opens a dialog box asking the user to approve or deny the attempt, and 
 optionally remember the decision for the future. This patch moves the 
 evaluation of this config option into the codepath taken by any app 
 authorization check, in effect turning it into a Prompt when an application 
 accesses a wallet setting.
 
 The purpose is to allow distributions such as Kubuntu and Netrunner which 
 want to make KWallet mostly invisible during routine operations to disable 
 this setting by default and so avoid the user being prompted to grant 
 applications wallet access rights in more situations. (It should be pointed 
 out that application identity is apparently based on KAboutData information 
 anyway, and so the security of this system is dubious to begin with.)
 
 
 In the interest of keeping the delta between upstream and downstream as small 
 as possible I'd say it makes sense to pick this up.
 
 
 This diff is to be applied after the diff in: 
 https://git.reviewboard.kde.org/r/110328/
 
 A patch rewording the checkbox label in kwalletmanager has been posted for 
 review here: https://git.reviewboard.kde.org/r/110331/
 
 
 Diffs
 -
 
   kwalletd/kwalletd.cpp fa9fc11 
 
 Diff: http://git.reviewboard.kde.org/r/110330/diff/
 
 
 Testing
 ---
 
 Test package for Kubuntu by Harald Sitter, operation verified at runtime.
 
 
 Thanks,
 
 Eike Hein
 




  1   2   3   >