Re: Licensing for cxx-kde-frameworks (Rust bridge for KF6)

2024-11-01 Thread Albert Astals Cid
El dijous, 31 d’octubre del 2024, a les 13:47:05 (Hora estàndard del Centre 
d’Europa), Sune Vuorela va escriure:
> On 2024-10-30, Darshan Phaldesai  wrote:
> > Few considerations:
> > First, I plan to include bridges for most libraries needed for
> > application development and so need to consider their licenses as well.
> > Second, This project will only hold the "bridge" code and thus users
> > will still need to install the libraries. I don't think this should
> > cause any license violations but the bridge code is based of the method
> > signatures of the original libraries.
> > Third, Upstream `cxx-qt` project uses Apache-2.0+MIT and I planned to do
> > the same but KDE's Licensing policy doesn't mention any thing about
> > Apache-2.0.
> 
> I'd say just stick the same license on it as the KDE Frameworks in
> question.
> Given they are a directly derived work and also uses the KDE Frameworks
> underneath, giving any other license is just going to be confusing for
> the people involved.

+1

Cheers,
  Albert

> 
> The app developers needs to deal with (l)gpl licenses anyway.
> 
> /Sune






Re: Crow Translate

2024-07-19 Thread Albert Astals Cid
El dimecres, 17 de juliol del 2024, a les 23:46:18 (CEST), Hennadii 
Chernyshchyk va escriure:
> Thanks for the feedback!
> Added to the application as well:
> https://invent.kde.org/office/crow-translate/-/merge_requests/732
> Feel free to comment on the PR.

I would have really preferred if the "Don't show again" was not ticked by 
default, but if noone else complains, i guess it's fine.

Cheers,
  Albert

> 
> ср, 17 июл. 2024 г. в 21:58, :
> > On 2024-07-17 17:07, Hennadii Chernyshchyk wrote:
> > > Added it to the readme and to the app metadata.
> > > Unsure about adding it to the application as well, I don't like
> > > popups, even one-timers...
> > > But I will add it to the app if you or anyone else will insist on it.
> > 
> > Hi,
> > 
> > I think it should be in the app, either as inline message widget or
> > popup,
> > one can not expect that users need to read the meta data somewhere.
> > 
> > We try to be privacy sensitive, once telling that the data will float
> > around the
> > internet is really important in my eyes.
> > 
> > Greetings
> > Christoph






Re: Crow Translate

2024-07-16 Thread Albert Astals Cid
El divendres, 12 de juliol del 2024, a les 0:17:35 (CEST), Hennadii 
Chernyshchyk va escriure:
> > Thanks for the report, I will take a look!
> 
> Fixed!
> 
> 
> Regardless of the warning. I think that the only translation engine that
> fully respects privacy is LibreTranslate.
> But it doesn't support automatic language detection and can't do TTS. It's
> also disabled on the main Mozhi instance (instances can disable specific
> engines if they want).
> So I can add a warning, but there is not much to choose from :(
> Do you still think it's a good idea?

Yes, the warning is about making the user understand their data is leaving 
their computer and going into someone elses computer and we can't control what 
happens once its in someone elses computer.

Cheers,
  Albert

> 
> We can alternatively add this information to the project description in the
> appdata and README. And also inform that some instances can disable
> specific engines. What do you think about it?
> 
> чт, 11 июл. 2024 г. в 01:38, Hennadii Chernyshchyk :
> > > Where it says "Traducció auto" it is just "Traducció automàtica" but the
> > 
> > text is cut.
> > 
> > Thanks for the report, I will take a look!
> > 
> > > But which one are we using?
> > 
> > We pick a random default instance to spread the load. But users can set a
> > preferred one in settings.
> > 
> > > Could I ask for a dialog that has a "don't show again" check and the
> > 
> > check is not checked?
> > 
> > Makes sense, I will add!
> > 
> > чт, 11 июл. 2024 г. в 01:27, Albert Astals Cid :
> >> El dimarts, 9 de juliol del 2024, a les 0:48:12 (CEST), Hennadii
> >> Chernyshchyk
> >> 
> >> va escriure:
> >> > > No I mean link to BrezeIcons library.
> >> > 
> >> > Ah, I didn't know that there was a library for it! Will change it,
> >> > thank
> >> > you.
> >> > 
> >> > > The problem is that the defaultLocale() function returns the C
> >> > > QLocale
> >> > 
> >> > which is wrong since that's not the default locale on my user.
> >> > 
> >> > I never apply QLocale::c(). I see how it could be confusing, but I
> >> 
> >> actually
> >> 
> >> > use it as a special value to apply a system locale here:
> >> https://invent.kde.org/office/crow-translate/-/blob/master/src/settings/a
> >> pps>> 
> >> > ettings.cpp#L63 (I may need to refactor this part)
> >> 
> >> Fixed in 62e49303157cdfdf2758b09ff7b4530ff1eb921c :)
> >> 
> >> Possibly related to translations, the sizing could be a bit better, see
> >> https://i.imgur.com/wvSrGc0.png
> >> 
> >> Where it says "Traducció auto" it is just "Traducció automàtica" but the
> >> text
> >> is cut. You can see the text if you make the window wider.
> >> 
> >> This is not critical, but would be nice that a user doesn't get cut text
> >> on
> >> startup.
> >> 
> >> > By any chance you run a flatpak? If yes, I know about this issue and I
> >> 
> >> am
> >> 
> >> > working on it right now:
> >> > https://invent.kde.org/office/crow-translate/-/merge_requests/724.
> >> > For some reason it doesn't work when packaged as flatpak, I am
> >> > investigating it right now.
> >> > 
> >> > > Self hosted by who?
> >> > 
> >> > Can be self-hosted by anyone. Here is the list of known instances:
> >> > https://codeberg.org/aryak/mozhi#instances
> >> 
> >> But which one are we using?
> >> 
> >> Or is it a random one each time?
> >> 
> >> > > How does that matter?
> >> > > If i add the text of my potentially Nobel winning research for
> >> > 
> >> > translation, it
> >> > 
> >> > > will still end up in NonFreeSoftwareService and
> >> > > NonFreeSoftwareService
> >> > 
> >> > can
> >> > 
> >> > > read it and potentially steal it.
> >> > 
> >> > Ah, it's a valid concern.
> >> > What I tried to say is that it doesn't spy on specific users since data
> >> > basically goes through an instance instead of talking to a non-free
> >> 
> >> service
> >> 
> >> > directly.
> >> > Would

Re: Crow Translate

2024-07-16 Thread Albert Astals Cid
El diumenge, 14 de juliol del 2024, a les 2:31:00 (CEST), Kevin Kofler va 
escriure:
> Hennadii Chernyshchyk wrote:
> > Regardless of the warning. I think that the only translation engine that
> > fully respects privacy is LibreTranslate.
> > But it doesn't support automatic language detection and can't do TTS. It's
> > also disabled on the main Mozhi instance (instances can disable specific
> > engines if they want).
> > So I can add a warning, but there is not much to choose from :(
> > Do you still think it's a good idea?
> 
> The issue is inherently that your application is relying on a web service
> (Mozhi) to do the translations. That service then either forwards the
> requests to yet another (typically proprietary) web service (e.g., Google
> Translate) or to something local to the Mozhi server instance (e.g., a
> LibreTranslate instance running on the same server as the Mozhi instance).
> In the first case, there are two services that see your data, in the second
> case, there is still one service that sees your data and that you have to
> trust.
> 
> The only thing that is guaranteed to be privacy-friendly is to run (either
> as a separate process or embedded as a library) a local instance of
> something like LibreTranslate (or Argos Translate directly) or Bergamot (or
> Marian directly) on the same device as your application. And libmozhi is
> probably not the library you would want to use for that (though technically
> I guess you could theoretically run and point libmozhi to a local Mozhi
> instance and in turn point that to a local LibreTranslate instance).
> 
> As for:
> > But it doesn't support automatic language detection and can't do TTS.
> 
> there are separate libraries for that.
> 
> For automatic language detection, see, e.g., cld2 or cld3, or one of several
> Python or Perl modules.
> 
> For TTS, see eSpeak NG (or its library libespeakng) if you want something
> simple and compact that supports many languages out of the box, or one of
> the local modern AI-based TTS tools (Mozilla TTS, OpenAI Whisper, etc.) if
> you want something that sounds better.
> 
> And, while you did not ask for it, for STT, there is the old PocketSphinx,
> the newer Mozilla DeepSpeech, etc.
> 
> The Free Software implementations that you can run locally all focus on
> doing one thing well, not on offering a complete solution like the Google
> Translate web service does. Integrating that all into a complete solution is
> then your job as the application developer.

You're asking them to do a totally different application of the application 
that they are doing, that makes no sense.

Cheers,
  Albert

> 
> Kevin Kofler






Re: Crow Translate

2024-07-10 Thread Albert Astals Cid
El dimarts, 9 de juliol del 2024, a les 0:48:12 (CEST), Hennadii Chernyshchyk 
va escriure:
> > No I mean link to BrezeIcons library.
> 
> Ah, I didn't know that there was a library for it! Will change it, thank
> you.
> 
> > The problem is that the defaultLocale() function returns the C QLocale
> 
> which is wrong since that's not the default locale on my user.
> 
> I never apply QLocale::c(). I see how it could be confusing, but I actually
> use it as a special value to apply a system locale here:
> https://invent.kde.org/office/crow-translate/-/blob/master/src/settings/apps
> ettings.cpp#L63 (I may need to refactor this part)

Fixed in 62e49303157cdfdf2758b09ff7b4530ff1eb921c :)

Possibly related to translations, the sizing could be a bit better, see 
https://i.imgur.com/wvSrGc0.png

Where it says "Traducció auto" it is just "Traducció automàtica" but the text 
is cut. You can see the text if you make the window wider.

This is not critical, but would be nice that a user doesn't get cut text on 
startup. 

> 
> By any chance you run a flatpak? If yes, I know about this issue and I am
> working on it right now:
> https://invent.kde.org/office/crow-translate/-/merge_requests/724.
> For some reason it doesn't work when packaged as flatpak, I am
> investigating it right now.
> 
> > Self hosted by who?
> 
> Can be self-hosted by anyone. Here is the list of known instances:
> https://codeberg.org/aryak/mozhi#instances

But which one are we using?

Or is it a random one each time?

> 
> > How does that matter?
> > If i add the text of my potentially Nobel winning research for
> 
> translation, it
> 
> > will still end up in NonFreeSoftwareService and NonFreeSoftwareService
> 
> can
> 
> > read it and potentially steal it.
> 
> Ah, it's a valid concern.
> What I tried to say is that it doesn't spy on specific users since data
> basically goes through an instance instead of talking to a non-free service
> directly.
> Would you prefer to add a warning on a first startup?


A warning on first startup sounds good to me :) Could I ask for a dialog that 
has a "don't show again" check and the check is not checked?  

Cheers,
  Albert

> 
> вт, 9 июл. 2024 г. в 01:26, Albert Astals Cid :
> > El dimarts, 9 de juliol del 2024, a les 0:12:48 (CEST), Hennadii
> > Chernyshchyk
> > 
> > va escriure:
> > > Thanks for looking into the app!
> > > 
> > > > Please remove the submodule of breeze-icons and replace it by a
> > > 
> > > dependency to breeze-icons.
> > > 
> > > Could you elaborate? You mean to use something like FetchContent from
> > 
> > CMake?
> > 
> > > I use it to bundle icons for Windows since on Windows there are no icon
> > > themes.
> > 
> > No I mean link to BrezeIcons library.
> > 
> > > > Translations are not loaded properly. (i.e. i get the UI in English
> > > 
> > > instead of
> > > Catalan)
> > > 
> > > Are you running the application from the build directory? If yes, then
> > 
> > you
> > 
> > > won't have translation because the application expects them to be
> > > located
> > > in a system path. I have an application from the latest commit installed
> > > system-wide and translations work.
> > > I can tweak it to make it work from the build directory as well by
> > 
> > looking
> > 
> > > into the ECMPoQm folder.
> > 
> > The problem is that the defaultLocale() function returns the C QLocale
> > which
> > is wrong since that's not the default locale on my user.
> > 
> > > > I would appreciate a big warning when using non-free software engines
> > > 
> > > All engines use Mozhi which is self-hosted.
> > 
> > Self hosted by who?
> > 
> > > Yes, it talks to non-free
> > > software engines, but it's similar to Invidious or Piped - all
> > > communication is done through a user-choosed instance.
> > 
> > How does that matter?
> > 
> > If i add the text of my potentially Nobel winning research for
> > translation, it
> > will still end up in NonFreeSoftwareService and NonFreeSoftwareService can
> > read it and potentially steal it.
> > 
> > Cheers,
> > 
> >   Albert
> >   
> > > I'm fine with adding a warning, but I also think that privacy is
> > > somewhat
> > > protected.
> > > 
> > > вт, 9 июл. 2024 г. в 00:53, Albert Astals Cid :
> > > > El dissabte, 6 de 

Re: Crow Translate

2024-07-10 Thread Albert Astals Cid
El dimarts, 9 de juliol del 2024, a les 11:48:14 (CEST), Hennadii Chernyshchyk 
va escriure:
> I looked into KF6BreezeIcons library, but it requires Qt6, while my
> application is Qt5 for now.
> I planned to migrate to Qt6, but after the incubation process.
> Any chance we could keep the current way of bundling Breeze icons and
> migrate to KF6BreezeIcons after the Qt6 migration?

Right forgot that you're on Qt5, that's fine for me.

Cheers,
  Albert

> 
> вт, 9 июл. 2024 г. в 02:14, Hennadii Chernyshchyk :
> > Just fixed locales (as well as other issues related to it), the problem
> > was in my Flatpak configuration:
> > https://invent.kde.org/office/crow-translate/-/commit/4a31b516292455e9e905
> > a905f69101865fddb823
> > 
> > If you use it, please use the latest version.
> > 
> > вт, 9 июл. 2024 г. в 01:48, Hennadii Chernyshchyk :
> >> > No I mean link to BrezeIcons library.
> >> 
> >> Ah, I didn't know that there was a library for it! Will change it, thank
> >> you.
> >> 
> >> > The problem is that the defaultLocale() function returns the C QLocale
> >> 
> >> which is wrong since that's not the default locale on my user.
> >> 
> >> I never apply QLocale::c(). I see how it could be confusing, but I
> >> actually use it as a special value to apply a system locale here:
> >> https://invent.kde.org/office/crow-translate/-/blob/master/src/settings/a
> >> ppsettings.cpp#L63 (I may need to refactor this part)
> >> 
> >> By any chance you run a flatpak? If yes, I know about this issue and I am
> >> working on it right now:
> >> https://invent.kde.org/office/crow-translate/-/merge_requests/724.
> >> For some reason it doesn't work when packaged as flatpak, I am
> >> investigating it right now.
> >> 
> >> > Self hosted by who?
> >> 
> >> Can be self-hosted by anyone. Here is the list of known instances:
> >> https://codeberg.org/aryak/mozhi#instances
> >> 
> >> > How does that matter?
> >> > If i add the text of my potentially Nobel winning research for
> >> 
> >> translation, it
> >> 
> >> > will still end up in NonFreeSoftwareService and NonFreeSoftwareService
> >> 
> >> can
> >> 
> >> > read it and potentially steal it.
> >> 
> >> Ah, it's a valid concern.
> >> What I tried to say is that it doesn't spy on specific users since data
> >> basically goes through an instance instead of talking to a non-free
> >> service
> >> directly.
> >> Would you prefer to add a warning on a first startup?
> >> 
> >> вт, 9 июл. 2024 г. в 01:26, Albert Astals Cid :
> >>> El dimarts, 9 de juliol del 2024, a les 0:12:48 (CEST), Hennadii
> >>> Chernyshchyk
> >>> 
> >>> va escriure:
> >>> > Thanks for looking into the app!
> >>> > 
> >>> > > Please remove the submodule of breeze-icons and replace it by a
> >>> > 
> >>> > dependency to breeze-icons.
> >>> > 
> >>> > Could you elaborate? You mean to use something like FetchContent from
> >>> 
> >>> CMake?
> >>> 
> >>> > I use it to bundle icons for Windows since on Windows there are no
> >>> > icon
> >>> > themes.
> >>> 
> >>> No I mean link to BrezeIcons library.
> >>> 
> >>> > > Translations are not loaded properly. (i.e. i get the UI in English
> >>> > 
> >>> > instead of
> >>> > Catalan)
> >>> > 
> >>> > Are you running the application from the build directory? If yes, then
> >>> 
> >>> you
> >>> 
> >>> > won't have translation because the application expects them to be
> >>> 
> >>> located
> >>> 
> >>> > in a system path. I have an application from the latest commit
> >>> 
> >>> installed
> >>> 
> >>> > system-wide and translations work.
> >>> > I can tweak it to make it work from the build directory as well by
> >>> 
> >>> looking
> >>> 
> >>> > into the ECMPoQm folder.
> >>> 
> >>> The problem is that the defaultLocale() function returns the C QLocale
> >>> which
> >>> is wrong since that's not the default locale on my 

Re: Crow Translate

2024-07-08 Thread Albert Astals Cid
El dimarts, 9 de juliol del 2024, a les 0:12:48 (CEST), Hennadii Chernyshchyk 
va escriure:
> Thanks for looking into the app!
> 
> > Please remove the submodule of breeze-icons and replace it by a
> 
> dependency to breeze-icons.
> 
> Could you elaborate? You mean to use something like FetchContent from CMake?
> I use it to bundle icons for Windows since on Windows there are no icon
> themes.

No I mean link to BrezeIcons library.

> 
> > Translations are not loaded properly. (i.e. i get the UI in English
> 
> instead of
> Catalan)
> 
> Are you running the application from the build directory? If yes, then you
> won't have translation because the application expects them to be located
> in a system path. I have an application from the latest commit installed
> system-wide and translations work.
> I can tweak it to make it work from the build directory as well by looking
> into the ECMPoQm folder.

The problem is that the defaultLocale() function returns the C QLocale which 
is wrong since that's not the default locale on my user.

> 
> > I would appreciate a big warning when using non-free software engines
> 
> All engines use Mozhi which is self-hosted. 

Self hosted by who?

> Yes, it talks to non-free
> software engines, but it's similar to Invidious or Piped - all
> communication is done through a user-choosed instance.

How does that matter?

If i add the text of my potentially Nobel winning research for translation, it 
will still end up in NonFreeSoftwareService and NonFreeSoftwareService can 
read it and potentially steal it.

Cheers,
  Albert

> I'm fine with adding a warning, but I also think that privacy is somewhat
> protected.
> 
> вт, 9 июл. 2024 г. в 00:53, Albert Astals Cid :
> > El dissabte, 6 de juliol del 2024, a les 0:57:22 (CEST), Hennadii
> > Chernyshchyk
> > 
> > va escriure:
> > > Hi!
> > > 
> > > I'm one of the developers of Crow Translate
> > > <https://invent.kde.org/office/crow-translate>, a translator app that
> > 
> > uses
> > 
> > > Mozhi <https://codeberg.org/aryak/mozhi>. No, the app can't translate
> > 
> > crow
> > 
> > > calls - that's just the name :D
> > > But you can quickly translate text from selection or from the screen. We
> > > also provide a CLI app and D-Bus API for translation automation. The app
> > 
> > is
> > 
> > > written in QtWidgets, but I made it adaptive. On linuxphoneapps.org
> > > <https://linuxphoneapps.org/apps/io.crow_translate.crowtranslate/> it's
> > > rated 5 for mobile fit (I use it on my PinePhone Pro).
> > > 
> > > Crow Translate has recently been incubated and is now looking to become
> > 
> > an
> > 
> > > official part of KDE. For this we are now in KDE Review and have already
> > > worked through the checklist in #699. Please make sure you are fine with
> > > the current state of Crow Translate being released as a KDE project,
> > > because we plan on moving on to making a release soon after the KDE
> > 
> > review
> > 
> > > time period has completed.
> > 
> > Please remove the submodule of breeze-icons and replace it by a dependency
> > to
> > breeze-icons.
> > 
> > The other submodules would ideally also go away but submoduling our own
> > things
> > is a no go.
> > 
> > Translations are not loaded properly. (i.e. i get the UI in English
> > instead of
> > Catalan)
> > 
> > I would appreciate a big warning when using non-free software engines that
> > there's no expectation of privacy on what is being translated (and same on
> > the
> > Free Software ones if they don't promise that level of privacy).
> > 
> > Cheers,
> > 
> >   Albert






Re: Crow Translate

2024-07-08 Thread Albert Astals Cid
El dissabte, 6 de juliol del 2024, a les 0:57:22 (CEST), Hennadii Chernyshchyk 
va escriure:
> Hi!
> 
> I'm one of the developers of Crow Translate
> , a translator app that uses
> Mozhi . No, the app can't translate crow
> calls - that's just the name :D
> But you can quickly translate text from selection or from the screen. We
> also provide a CLI app and D-Bus API for translation automation. The app is
> written in QtWidgets, but I made it adaptive. On linuxphoneapps.org
>  it's
> rated 5 for mobile fit (I use it on my PinePhone Pro).
> 
> Crow Translate has recently been incubated and is now looking to become an
> official part of KDE. For this we are now in KDE Review and have already
> worked through the checklist in #699. Please make sure you are fine with
> the current state of Crow Translate being released as a KDE project,
> because we plan on moving on to making a release soon after the KDE review
> time period has completed.


Please remove the submodule of breeze-icons and replace it by a dependency to 
breeze-icons.

The other submodules would ideally also go away but submoduling our own things 
is a no go.

Translations are not loaded properly. (i.e. i get the UI in English instead of 
Catalan)

I would appreciate a big warning when using non-free software engines that 
there's no expectation of privacy on what is being translated (and same on the 
Free Software ones if they don't promise that level of privacy).

Cheers,
  Albert




Re: KDE Builder: request for review

2024-04-09 Thread Albert Astals Cid
El dimarts, 9 d’abril del 2024, a les 12:51:50 (CEST), Sune Vuorela va 
escriure:
> On Mon, Apr 8, 2024 at 10:05 AM  wrote:
> > and deprecating the kdesrc-build. That way we can move forward with new
> > tool.
> I don't think reviewing or not of kde-builder should have any effect on
> kdesrc-build.

Yes, i think the original email comes from a bit of misunderstanding on how we 
work.

We are not a top-down organization.

There is hardly an "official" anything in KDE. 

We have dolphin but we also have krusader, sure one ships with KDE Gear and 
the other doesn't but that's more a release schedule decision than one being 
more official than the other.

We have gwenview and koko, both shipping in KDE Gear.

And more and more examples.

So unless the people working on kdesrc-build want people to stop 
using that and no one else steps up to work on it kdesrc-build will be as 
"official" as it is today, 0% official and 100% official at the same time.

Cheers,
  Albert

> 
> I think also we should be slightly wary of changing a long-running 20y
> old tool written by long time kde people who are still around with a
> brand new tool written by a brand new contributor.
> I wonder if it will have the same shelf life as the ruby version that
> was done some years ago and abandoned.
> 
> /Sune






Re: KDE Review: Skladnik (t.g.f.k.a. KSokoban), returning a KDE1-KDE3 age dino

2024-01-29 Thread Albert Astals Cid
El dilluns, 29 de gener de 2024, a les 10:09:07 (CET), Jonathan Riddell va 
escriure:
> On Sat, 27 Jan 2024 at 17:26, Friedrich W. H. Kossebau 
> 
> wrote:
> > [ ] App packages in Flatpak, Snap, AppImages and Windows etc as
> > appropriate
> > 
> > Not involved with any of those packages/platforms, but I assume that
> > checkbox
> > is rather some kind of "nice to have", not blocking the process actually?
> 
> This sort of comment  makes me really sad. The All About the Apps goal,
> which in principle is still ongoing, was an attempt to get KDE developers
> to realise it was important not just to write apps but to actually make
> them available to users, I find it astonishing how we still don't have a
> culture where making our apps available to users is part of our
> responsibility.  There's teams in KDE to help with all these formats.
> Sorry to complain here as the issue is larger than just this one app but
> it's so sad that nobody within KDE wants to help get users using our
> software directly.

Please don't exaggerate, "nobody" is a very strong word and you very well know 
it's not true.

Best Regards,
  Albert

> 
> Jonathan






Re: Kandalf: request for review

2023-12-07 Thread Albert Astals Cid
El divendres, 8 de desembre de 2023, a les 0:44:11 (CET), Carl Schwan va 
escriure:
> On Fri, Dec 8, 2023, at 12:10 AM, Albert Astals Cid wrote:
> > El dijous, 7 de desembre de 2023, a les 16:28:48 (CET), Loren Burkholder
> > va
> > 
> > escriure:
> > > Hi all!
> > > 
> > > I've been working towards getting Kandalf ready for integration into KDE
> > > (and I'd like to thank everyone who has pitched in to help me, mainly
> > > Laurent and redstrate). At this point, while the app is not necessarily
> > > feature-complete or fully polished, it's getting close to that point.
> > > Therefore, I'd like to officially request a review of Kandalf for
> > > inclusion
> > > in the KDE apps.
> > 
> > Where is the thing we're supposed to review?
> 
> In the wrong place: https://invent.kde.org/lorendb/kandalf/-/issues/1

And again we have people self-checking the checkboxes or their own review 
:sad_dance:

> I asked Loren to create a sysadmin ticket to get it moved to an offical
> namespace.

That seems premature? What if the review never succeeds?

Cheers,
  Albert

> 
> Cheers,
> Carl






Re: Kandalf: request for review

2023-12-07 Thread Albert Astals Cid
El dijous, 7 de desembre de 2023, a les 16:28:48 (CET), Loren Burkholder va 
escriure:
> Hi all!
> 
> I've been working towards getting Kandalf ready for integration into KDE
> (and I'd like to thank everyone who has pitched in to help me, mainly
> Laurent and redstrate). At this point, while the app is not necessarily
> feature-complete or fully polished, it's getting close to that point.
> Therefore, I'd like to officially request a review of Kandalf for inclusion
> in the KDE apps.

Where is the thing we're supposed to review?

Cheers,
  Albert

> 
> You can reach me on Matrix at @lorendb:nheko.im for more instant
> communications.
> 
> Thanks,
> Loren Burkholder






Re: KDE Review: Hash-o-Matic

2023-10-02 Thread Albert Astals Cid
El dilluns, 2 d’octubre de 2023, a les 22:55:05 (CEST), Nate Graham va 
escriure:
> On 10/2/23 13:53, Albert Astals Cid wrote:
> > El diumenge, 1 d’octubre de 2023, a les 21:49:36 (CEST), Carl Schwan va
> > Who checked all those marks? There's no way to know.
> 
> If you scroll down to "Activity", it says who checked them after the
> issue was opened.

When i complained [almost] all the checkmarks where checked and it didn't say 
anything about who checked them.

See how there's a 

"marked the checklist item Passing CI job for Reuse linting as incomplete"

but there's no "marked as complete" before it.

> 
> I agree that the person who opened the Issue should not check anything
> themselves before opening the Issue up, though. This seems like a
> sensible policy.
> 
> Nate






Re: KDE Review: Hash-o-Matic

2023-10-02 Thread Albert Astals Cid
El diumenge, 1 d’octubre de 2023, a les 21:49:36 (CEST), Carl Schwan va 
escriure:
> Hello,
> 
> I started writting a small application to generate and compare files with
> their checksum two years. I piked it up again recently and I think this is
> now ready for a kde review.
> 
> Features includes:
>  - Generate checksum from a file
>  - Compare two files
>  - Verify a file with a given checksum
>  - Verify a file with a given .sig file with GPG and show the signature info
> 
> Here is the kde review checklist:
> https://invent.kde.org/utilities/hash-o-matic/-/issues/1
> 
> It would be great if someone could create a product on bugs.kde.org and
> assign myself to the product.

This method of review is really sub-optiomal.

Who checked all those marks? There's no way to know.

Was it someone expert in the area? 

Was it someone that knows has no idea what the checks mean?

Or was it the submitter of review? If it's the submitter for review it's 
worthless (nothing against Carl, you're great) but one doesn't review their 
own MRs, so one shouldn't probably review this kind of checks either.

I'm abstaining from this review and any coming reviews due to the medium being 
worse that the old "Have an email thread on k-c-d".

Cheers,
  Albert

> 
> Cheers,
> Carl






Re: KCoreDirLister::setMimeExcludeFilter()

2023-09-17 Thread Albert Astals Cid
El diumenge, 16 de juliol de 2023, a les 8:46:44 (CEST), Albert Astals Cid va 
escriure:
> El dissabte, 29 d’abril de 2023, a les 12:17:06 (CEST), Albert Astals Cid va
> escriure:
> > El dimecres, 26 d’abril de 2023, a les 18:09:44 (CEST), Jonathan Marten va
> > 
> > escriure:
> > > Hello,
> > > 
> > > This has been deprecated in 5.100 with the comment "no known users".
> > > Unfortunately there is a use in Kooka, where it is used to configure a
> > > KDirOperator (via its KDirLister) to only show files and not
> > > directories.  This happens at
> > > 
> > > https://invent.kde.org/graphics/kooka/-/blob/master/app/thumbview.cpp#L8
> > > 4
> > > 
> > > Although KDirOperator has setMode(KFile::Modes), setting this to
> > > KFile::File or KFile::Files seems to only affect the selection mode and
> > > does not stop directories from being shown.
> > > 
> > > The API doc for KCoreDirLister::setMimeExcludeFilter() says "Filtering
> > > should be done with KFileFilter", but from the current KFileFilter doc
> > > it appears that this will only be able to be an "include this file or
> > > MIME type type" filter, not an "exclude" one.
> > > 
> > > Is there currently - or will there be in the future - a way to configure
> > > a KDirOperator in a "files only" mode, in the same way as "directories
> > > only" is currently supported?  If it turns out that
> > > KDirOperator::setMode()
> > > is intended to be have that effect, then I'd be happy to investigate why
> > > it doesn't.
> > 
> > I guess we should report that deprecation since the assumpion of the
> > deprecation is just wrong.
> > 
> > Nico any reason against bringing it back?
> 
> Please see https://invent.kde.org/frameworks/kio/-/merge_requests/1300

We have now brought it back both in kf6 and kf5 branches.

Cheers,
  Albert

> 
> Cheers,
>   Albert
> 
> > Cheers,
> > 
> >   Albert
> >   
> > > Best regards,






Re: drkonqi's many debuggers

2023-09-04 Thread Albert Astals Cid
El dijous, 31 d’agost de 2023, a les 12:42:59 (CEST), Halla Rempt va escriure:
> On dinsdag 29 augustus 2023 05:22:56 CEST Thiago Macieira wrote:
> > True, but the majority of our user base is still Linux, so if we had to
> > choose only one, it would have to be gdb.
> 
> Um, no? The majority is on Windows.

Do you have actual numbers for this?

Albert

> 
> Halla






Re: KCoreDirLister::setMimeExcludeFilter()

2023-07-15 Thread Albert Astals Cid
El dissabte, 29 d’abril de 2023, a les 12:17:06 (CEST), Albert Astals Cid va 
escriure:
> El dimecres, 26 d’abril de 2023, a les 18:09:44 (CEST), Jonathan Marten va
> 
> escriure:
> > Hello,
> > 
> > This has been deprecated in 5.100 with the comment "no known users".
> > Unfortunately there is a use in Kooka, where it is used to configure a
> > KDirOperator (via its KDirLister) to only show files and not
> > directories.  This happens at
> > 
> > https://invent.kde.org/graphics/kooka/-/blob/master/app/thumbview.cpp#L84
> > 
> > Although KDirOperator has setMode(KFile::Modes), setting this to
> > KFile::File or KFile::Files seems to only affect the selection mode and
> > does not stop directories from being shown.
> > 
> > The API doc for KCoreDirLister::setMimeExcludeFilter() says "Filtering
> > should be done with KFileFilter", but from the current KFileFilter doc
> > it appears that this will only be able to be an "include this file or
> > MIME type type" filter, not an "exclude" one.
> > 
> > Is there currently - or will there be in the future - a way to configure
> > a KDirOperator in a "files only" mode, in the same way as "directories
> > only" is currently supported?  If it turns out that
> > KDirOperator::setMode()
> > is intended to be have that effect, then I'd be happy to investigate why
> > it doesn't.
> 
> I guess we should report that deprecation since the assumpion of the
> deprecation is just wrong.
> 
> Nico any reason against bringing it back?

Please see https://invent.kde.org/frameworks/kio/-/merge_requests/1300

Cheers,
  Albert

> 
> Cheers,
>   Albert
> 
> > Best regards,






Re: XWayland Video Bridge in kdereview

2023-06-16 Thread Albert Astals Cid
El divendres, 16 de juny de 2023, a les 18:43:59 (CEST), Aleix Pol va 
escriure:
> Hi Everyone,
> As discussed by david in his blog post [1], we worked on the component
> in question to solve the problem of X11 applications that want to use
> the screencasting services on Wayland.
> 
> https://invent.kde.org/system/xwaylandvideobridge
> 
> While there's some corners to polish here and there, it's ready to be
> used by everyone who needs it. In fact, people are using it from the
> flatpak and distros want to ship it so we should probably make it
> easier and regulated through a proper release.

CI seems borked 

https://invent.kde.org/system/xwaylandvideobridge/-/merge_requests/9

Cheers,
  Albert

> 
> First it should go through kdereview, then I'd suggest starting to
> release it with Plasma 5.27 in one of the minor releases. I don't
> think it's something we have done before but it doesn't add new
> dependencies and it has an impact in our product offering which is,
> incidentally, frozen due to the switch to *6.
> 
> I would appreciate the review of the module, I guess the "how to
> release it" is something we can discuss in a couple of weeks once this
> is sorted.
> 
> Thanks everyone!
> Aleix
> 
> [1] https://blog.davidedmundson.co.uk/blog/xwaylandvideobridge/






Re: sentry evaluation

2023-06-04 Thread Albert Astals Cid
El dijous, 1 de juny de 2023, a les 12:35:41 (CEST), Harald Sitter va 
escriure:
> Hey,
> 
> We've had almost a year, albeit in a super limited capacity for git
> builds, with sentry (https://errors-eval.kde.org) and we should
> probably render a final verdict on the evaluation.
> 
> Did you like it?

Did I miss the email announcing this?

I can only see a "Towards Excellent Defect Management" sent 2 years ago that 
describes a plan, but i can't see any kind of "You can join sentry to do this 
and that".

By clicking on https://errors-eval.kde.org i can't really seem to see anything 
to evaluate either, all it deos is it asks me to join a team.

Cheers,
  Albert

> What could be improved?
> Should we move ahead with a more permanent setup?
> 
> The overall game plan would be to have drkonqi ask the user to opt
> into automatic crash submission when they open drkonqi so we get close
> to all crashes (the ones caught by kcrash) automatically filed into
> sentry from then on out. To increase exposure of this feature I'd also
> like to glue it into the feedback KCM but I'm not yet sure if it
> should be a separate setting or linked to the regular feedback
> categories, the former sounds more accessible. The current bugzilla
> based workflow would still be available for when the user actually
> wants to write a description.
> 
> (previous discussion: https://markmail.org/thread/6y5paczdposz3aoj)
> 
> HS






Re: KCoreDirLister::setMimeExcludeFilter()

2023-04-29 Thread Albert Astals Cid
El dimecres, 26 d’abril de 2023, a les 18:09:44 (CEST), Jonathan Marten va 
escriure:
> Hello,
> 
> This has been deprecated in 5.100 with the comment "no known users".
> Unfortunately there is a use in Kooka, where it is used to configure a
> KDirOperator (via its KDirLister) to only show files and not
> directories.  This happens at
> 
> https://invent.kde.org/graphics/kooka/-/blob/master/app/thumbview.cpp#L84
> 
> Although KDirOperator has setMode(KFile::Modes), setting this to
> KFile::File or KFile::Files seems to only affect the selection mode and
> does not stop directories from being shown.
> 
> The API doc for KCoreDirLister::setMimeExcludeFilter() says "Filtering
> should be done with KFileFilter", but from the current KFileFilter doc
> it appears that this will only be able to be an "include this file or
> MIME type type" filter, not an "exclude" one.
> 
> Is there currently - or will there be in the future - a way to configure
> a KDirOperator in a "files only" mode, in the same way as "directories
> only" is currently supported?  If it turns out that KDirOperator::setMode()
> is intended to be have that effect, then I'd be happy to investigate why
> it doesn't.

I guess we should report that deprecation since the assumpion of the 
deprecation is just wrong.

Nico any reason against bringing it back?

Cheers,
  Albert


> 
> Best regards,






Re: KSvg in kdereview

2023-04-23 Thread Albert Astals Cid
El dijous, 20 d’abril de 2023, a les 10:25:34 (CEST), Marco Martin va 
escriure:
> Hi all,
> A part of plasma-framewrok, which is the one to do SVG-based themes,
> has now been splitted in a standalone library which is intended to be
> a new framework in KF6 (all usages of the plasma-framework version
> will be ported once this officially lands, and then those classes will
> be removed)
> The repo for now lives in
> https://invent.kde.org/libraries/plasmasvg
> 
> In the end it will be renamed in ksvg
> 
> Comments? reviews?

The README.md needs to be updated i think.

Cheers,
  Albert





Re: KSvg in kdereview

2023-04-23 Thread Albert Astals Cid
El dijous, 20 d’abril de 2023, a les 10:25:34 (CEST), Marco Martin va 
escriure:
> Hi all,
> A part of plasma-framewrok, which is the one to do SVG-based themes,
> has now been splitted in a standalone library which is intended to be
> a new framework in KF6 (all usages of the plasma-framework version
> will be ported once this officially lands, and then those classes will
> be removed)
> The repo for now lives in
> https://invent.kde.org/libraries/plasmasvg
> 
> In the end it will be renamed in ksvg
> 
> Comments? reviews?

There's a few "TODO KF6" in the code, should those be addressed?

Is the plan to make it less plasma-specific with the rename?

i.e. the ImageSet constructor says

Default constructor. It will be the global theme configured in plasmarc

Is that wanted or not (i've no opinion, just asking to make sure since the 
rename seems to want to make it "less plasma").

Cheers,
  Albert





Re: Strange language setting issues

2023-03-22 Thread Albert Astals Cid
El dimecres, 22 de març de 2023, a les 21:38:58 (CET), Michael Reeves va 
escriure:
> I receiving complaints about French words appearing sporadically on
> KDiff3's GUi even when KDiff3 itself is set to  use English.
> I strongly suspect a setup issue within Windows but require assistance to
> rule out frameworks/qt involvement in the issue.
> https://bugs.kde.org/show_bug.cgi?id=442607
> 
> KDiff3 itself does not attempt to interfere with or alter the translation
> process in any way,

Forcing the language on the application settings (which seems to be what the 
user is doing) didn't work on Windows until very recently, you'll need a newer 
build that is based in KF 5.104.

Cheers,
  Albert





Re: RFC: an improved ki18n_install

2023-03-07 Thread Albert Astals Cid
El dimarts, 7 de març de 2023, a les 9:21:07 (CET), Milian Wolff va escriure:
> On Dienstag, 7. März 2023 02:51:06 CET Aleix Pol wrote:
> > On Mon, Mar 6, 2023 at 8:31 PM Milian Wolff  wrote:
> > > Hey all,
> > > 
> > > for context please see [1] and [2].
> > > 
> > > [1]: https://bugs.kde.org/show_bug.cgi?id=460245
> > > [2]:
> > > https://discourse.cmake.org/t/feature-request-add-custom-command-all/760
> > > 9
> > > 
> > > The gist is that me and Igor are annoyed by `ki18n_install` abusing
> > > `add_custom_target`, which makes the build always dirty. I.e. every time
> > > we
> > > run `ninja` we will, at the very least, run the two mo & ts generation
> > > targets. For kdevelop, these do non-trivial amounts of work that takes
> > > time
> > > even on a beefy laptop:
> > > 
> > > ```
> > > $ ninja && time ninja
> > > [2/2] Generating mo...
> > > [2/2] Generating mo...
> > > 
> > > real0m1,780s
> > > user0m5,742s
> > > sys 0m2,327s
> > > ```
> > > 
> > > I would like to fix this, but first want to get feedback from the KDE
> > > developers at large.
> > > 
> > > First of all: Are all projects using ki18n_install in the way we do? Why
> > > is
> > > no-one else complaining about this so far? Are your po files so small
> > > that
> > > this doesn't take a significant amount of time? Or are there potentially
> > > just so few of them in your project? KDevelop is heavily plugin based
> > > which means we have tons of po files. 2464 *.po files to be exact...
> > > 
> > > Secondly: does anyone know how to best approach a solution to this
> > > issue?
> > > The problem is that I don't see an easy way to solve it: While Kyle
> > > Edwards was nice enough to show me a way to tell CMake to not make the
> > > custom target always-dirty, `k18n_install` suffers from another design
> > > deficiency: It uses globbing internally and has an undefined amount of
> > > inputs and outputs, which basically makes it impossible for us to
> > > leverage `add_custom_command(OUTPUT` here, or?
> > > 
> > > As such, it seems like one would need a different approach to this
> > > problem
> > > anyways. Globbing is a known-evil in cmake land, and I would personally
> > > prefer to have the inputs explicitly listed, just like we do for source
> > > files etc. Because we have many languages though, listing all possible
> > > *.po or *.ts files is cumbersome. Instead, what about we maintain the
> > > list of known languages in the ki18n framework? Then we could have
> > > something like
> > > 
> > > ```
> > > ki18n_target(
> > > 
> > > # the target for which we are handling messages
> > > TARGET KF5I18n
> > > # the translation domain, added as compile_options and to find files
> > > TRANSLATION_DOMAIN ki18n5
> > > # the root po folder location, to look for the .po/.js files
> > > PO_FOLDER ${KI18n_SOURCE_DIR}/po
> > > 
> > > )
> > > ```
> > > 
> > > Internally, this would do what is currently done manually:
> > > 
> > > ```
> > > target_compile_options(KF5I18n PRIVATE -DTRANSLATION_DOMAIN=\"ki18n5\")
> > > ```
> > > 
> > > Then it would iterate over the list of known languages and try to find
> > > the
> > > .po or .js file. For every match, we would add_custom_target to generate
> > > the corresponding .mo/.ts file and add the reverse dependency.
> > > 
> > > What do others say to this approach?
> > > 
> > > Thanks
> > > 
> > > --
> > > Milian Wolff
> > > http://milianw.de
> > 
> > +1 to getting the problem solved. I've also been bothered by this and
> > thought about looking into this so thanks for stepping up! (also I'm
> > pretty sure this code is originally mine from a time when we didn't
> > generate translations on every single build ^^').
> > 
> > Having ninja/make do this dependency generation can end up being more
> > work than worth it because then we need to have a static list and then
> > we need to re-run it when the po files change and it's all a mess.
> 
> The only mess that I can see is having to maintain the static list.
> Otherwise, we would just piggy back on the underlying buildsystem to drive
> the generation for us. Meaning, we would get separate targets for every
> .po/.js, leading to proper parallelization too. And only new/changed files
> would get re-generated as needed.
> 
> Anyhow, reaping those benefits is going to be hard due to the static list.
> As Albert said in his email, my proposal has no advantages over just using
> glob. As such, we have basically three options:
> 
> a) status quo:
>  + trivial to setup
>  + will always correctly track changes to the .po/.js files
>  - serial instead of parallel generation of .mo/.ts files
>  - always dirty ALL target
> 
> b) glob with separate targets:
>  + trivial to setup
>  + parallelized generation of .mo/.ts files
>  + clean ALL target
>  - detecting new .po/.js files requires manual re-run of cmake (changes to
> existing .po/.js files will be detected automatically)
> 
> c) static list:
>  + parallelized generation of .mo/.t

Re: RFC: an improved ki18n_install

2023-03-06 Thread Albert Astals Cid
El dilluns, 6 de març de 2023, a les 20:31:01 (CET), Milian Wolff va escriure:
> Hey all,
> 
> for context please see [1] and [2].
> 
> [1]: https://bugs.kde.org/show_bug.cgi?id=460245
> [2]:
> https://discourse.cmake.org/t/feature-request-add-custom-command-all/7609
> 
> The gist is that me and Igor are annoyed by `ki18n_install` abusing
> `add_custom_target`, which makes the build always dirty. I.e. every time we
> run `ninja` we will, at the very least, run the two mo & ts generation
> targets. For kdevelop, these do non-trivial amounts of work that takes time
> even on a beefy laptop:
> 
> ```
> $ ninja && time ninja
> [2/2] Generating mo...
> [2/2] Generating mo...
> 
> real0m1,780s
> user0m5,742s
> sys 0m2,327s
> ```
> 
> I would like to fix this, but first want to get feedback from the KDE
> developers at large.
> 
> First of all: Are all projects using ki18n_install in the way we do? Why is
> no-one else complaining about this so far? Are your po files so small that
> this doesn't take a significant amount of time? Or are there potentially
> just so few of them in your project? KDevelop is heavily plugin based which
> means we have tons of po files. 2464 *.po files to be exact...
> 
> Secondly: does anyone know how to best approach a solution to this issue?
> The problem is that I don't see an easy way to solve it: While Kyle Edwards
> was nice enough to show me a way to tell CMake to not make the custom
> target always-dirty, `k18n_install` suffers from another design deficiency:
> It uses globbing internally and has an undefined amount of inputs and
> outputs, which basically makes it impossible for us to leverage
> `add_custom_command(OUTPUT` here, or?
> 
> As such, it seems like one would need a different approach to this problem
> anyways. Globbing is a known-evil in cmake land, and I would personally
> prefer to have the inputs explicitly listed, just like we do for source
> files etc. Because we have many languages though, listing all possible *.po
> or *.ts files is cumbersome. Instead, what about we maintain the list of
> known languages in the ki18n framework? Then we could have something like
> 
> ```
> ki18n_target(
> # the target for which we are handling messages
> TARGET KF5I18n
> # the translation domain, added as compile_options and to find files
> TRANSLATION_DOMAIN ki18n5
> # the root po folder location, to look for the .po/.js files
> PO_FOLDER ${KI18n_SOURCE_DIR}/po
> )
> ```
> 
> Internally, this would do what is currently done manually:
> 
> ```
> target_compile_options(KF5I18n PRIVATE -DTRANSLATION_DOMAIN=\"ki18n5\")
> ```
> 
> Then it would iterate over the list of known languages and try to find the
> .po or .js file. For every match, we would add_custom_target to generate
> the corresponding .mo/.ts file and add the reverse dependency.
> 
> What do others say to this approach?

How is globbing (checking what is in the filesystem) different from the 
checking 
against a known list that will check against the filesystem if a file exists?

Seems the same thing but with extra steps to me.

Cheers,
  Albert

> 
> Thanks






Re: KDE Review: Arianna

2023-02-26 Thread Albert Astals Cid
El diumenge, 26 de febrer de 2023, a les 4:53:05 (CET), Kevin Kofler va 
escriure:
> Carl Schwan wrote:
> > I want to move Arianna to KDE review. Arianna is an ebook reader
> > currently only supporting epubs. This is based on top of epub.js
> > and QtWebEngine for the actualy rendering of ebooks as doing that
> > from scratch in Qt would be too much work.
> 
> Okular has an EPub backend using libepub and QTextDocument. How does
> Arianna compare to that?

Okular is probably much worse, QTextDocument is not good for showing HTML-like 
content.

Cheers,
  Albert

> I would expect native code to be more efficient
> than JavaScript *especially* on mobile devices, which are apparently
> Arianna's main target. But I do not know whether there are, e.g., EPub
> documents that Arianna can render and Okular cannot, so that is why I am
> asking how they compare.
> 
> Kevin Kofler






Re: Proposal for using gitlab for kdereview process

2023-02-18 Thread Albert Astals Cid
El divendres, 17 de febrer de 2023, a les 8:53:54 (CET), David Redondo va 
escriure:
> Hi,
> 
> I observed that participation in kdereview is fairly low. Usually only the
> same few people participate (I have to admit this does not include me
> either). Further follow up to their comments tends sometimes to be slow or
> is missed. Sometimes it fizzles out completely.
> I think nowadays we have better tools to streamline the whole process, which
> we are familiar with and make the barrier to participation lower. My idea
> is to use Gitlab which we use for normal code review also for the kdereview
> process. For example it could be as follows
> - Announce the intention to go through kdereview as usual to kde-core-devel
> - Create a MR in your project's repo from master branch into an empty
> kdereview branch
> - Could copy the sanity checklist [1] as task list and check things there
> - Reviewers can leave review comments in the familiar web interface
> - Commits that fix review comments are pushed to master branch and so update
> the code seen in the MR
> - Once the reviewers are satisfied and their feedback incorporated, they
> approve the MR and it can be closed
> 
> Thoughts?

I'm not convinced more paper work is going to help us increase the number of 
participants in the review process (which is imho the big issue), but if 
you're willing to document all this properly I'm happy to give it a try and be 
proven wrong :)

Cheers,
  Albert



> 
> David
> 
> 
> 
> [1] https://community.kde.org/ReleasingSoftware#Sanity_Checklist






Re: New repo in kdereview: PlasmaTube

2023-02-05 Thread Albert Astals Cid
El dissabte, 4 de febrer de 2023, a les 19:14:20 (CET), Devin va escriure:
> Hi everyone,
> 
> I would like to put PlasmaTube through kdereview:
> 
> https://invent.kde.org/multimedia/plasmatube
> 
> PlasmaTube is a YouTube client for both mobile and desktop.

These two warnings seem like you should fix them
 qrc:/SettingsPage.qml:54: ReferenceError: logout is not defined
   There's no logout anywhere
 
 qrc:/videoplayer/VideoControls.qml:186: TypeError: Cannot read property 
'formatList' of undefined
   As far as I can see the video property of VideoControls is a MpvObject 
which doesn-t have a video property


KLocalizedString::setApplicationDomain("tokodon");
   This application is not tokodon ;)

You're also mixing tr() and i18n() in your C++ code, please move it all to 
i18n


Cheers,
  Albert


> 
> Thanks,
> Devin






Re: Request to relicense all CC0-1.0 code to MIT (or similar permissive license)

2023-01-22 Thread Albert Astals Cid
El diumenge, 22 de gener de 2023, a les 14:12:32 (CET), Neal Gompa va 
escriure:
> Hey folks,
> 
> During a review for flatpak-kcm for inclusion in Fedora, I discovered
> that KDE currently licenses its CI scripts under the CC0 (SPDX:
> CC0-1.0) license.

You mean .gitlab-ci.yml files ?

Just scrub those from the tarball if those are causing you problems since 
those are in no way needed for the build. 

If scrubing them from the released tarballs is a problem for some reason you 
could even try to propose changes to our packaging scripts that remove those 
from the release tarballs, if they are not super intrusive i guess that they 
could be accepted by the release folk.

Honestly, this seems more a Fedora problem than a KDE problem.

Cheers,
  Albert

> This is no longer generally permitted in Fedora for
> software/code due to the explicit exclusion of patent license
> grants[1].
> 
> While I realize that the likelihood of practical issues around this is
> pretty low for the case that KDE has been licensing the CI scripts
> under, it does cause headaches for us. I've made a request for a
> exception[2] so that we can continue to ship KDE software without
> having to take extraordinary actions, but I would like to request that
> KDE disallow code to be licensed under CC0-1.0 and recommend all
> existing code under that license to be transitioned to an accepted
> permissive license. My suggestion would be to recommend transitioning
> to the MIT license[3].
> 
> Thanks in advance and best regards,
> Neal
> 
> P.S.: I originally sent this to kde-licens...@kde.org, which
> apparently doesn't exist. Sorry for those CC'd getting this twice.
> Oops!
> 
> [1]:
> https://lists.fedoraproject.org/archives/list/le...@lists.fedoraproject.org
> /message/RRYM3CLYJYW64VSQIXY6IF3TCDZGS6LM/ [2]:
> https://lists.fedoraproject.org/archives/list/le...@lists.fedoraproject.org
> /message/Q67KFP4NT3WEL2Z75BYCXMP2IOQHLBJT/ [3]:
> https://opensource.org/licenses/MIT






Re: New santizer warning in KF 5.98 headers

2023-01-10 Thread Albert Astals Cid
El dimarts, 10 de gener de 2023, a les 22:49:43 (CET), Michael Reeves va 
escriure:
> /usr/include/KF5/KConfigWidgets/kstandardaction.h:261:64: runtime error:
> load of value 4294967295, which is not a valid value for type
> 'Qt::ConnectionType'
> 
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> /usr/include/KF5/KConfigWidgets/kstandardaction.h:261:64 in
> 
> The issue stems for assigning an int to a enum which is internally
> considered unsigned and possibly smaller than the four byte int. If this is
> doing what we expect than I need a way to shut off the warning.

That code has been there since May last year, so not exactly "new".

Given it doesn't seem to be crashing it would seem it's one of those "undefined 
but it works in all the compilers we care about".

Of course patches to make the sanitizer are really welcome :)

Cheers,
  Albert





Re: New repo in kdereview: kclock

2022-12-30 Thread Albert Astals Cid
El divendres, 30 de desembre de 2022, a les 8:46:56 (CET), Justin va escriure:
> Hey Team,
> 
> This was last posted about on October 18 2021. I looked at the three
> items that were needed for kirigami-addons:
> 
> * Missing is REUSE compliance:
> https://invent.kde.org/libraries/kirigami-addons/-/merge_requests/13
> * Hanyoung is working on a better time picker:
> https://invent.kde.org/libraries/kirigami-addons/-/merge_requests/17
> * And Clau is working on a better date picker:
> https://invent.kde.org/libraries/kirigami-addons/-/merge_requests/20
> 
> They have all now been merged. Can we resume the review on
> kirigami-addons so we can continue on kclock?
> 
> Thanks everyone and have a enjoyable and safe holiday!

I gave a long list of comments back in the day that were never answered, do 
that first and maybe i give it another look.

Cheers,
  Albert

> 
> Justin






Re: Docker CI Image Change Freeze

2022-12-12 Thread Albert Astals Cid
El dilluns, 12 de desembre de 2022, a les 18:28:08 (CET), Ben Cooksley va 
escriure:
> On Mon, Dec 12, 2022 at 12:29 PM Albert Astals Cid  wrote:
> > El dimarts, 29 de novembre de 2022, a les 10:15:33 (CET), Ben Cooksley va
> > 
> > escriure:
> > > Hi all,
> > > 
> > > As an update to this, sufficient changes have been made within Craft
> > > that
> > > it is now possible to build Qt 5 images so i'm releasing the freeze for
> > > those images.
> > > Qt 6 remains broken, and therefore remains frozen at this time (see
> > > https://invent.kde.org/sysadmin/ci-images/-/jobs/619808)
> > > 
> > > As mentioned previously, I believe this to be a CMake bug given the lack
> > 
> > of
> > 
> > > change in Ninja.
> > > 
> > > At this stage I would suggest investigation be focused on either
> > 
> > upgrading
> > 
> > > to a newer version of Ninja that can handle the newer version of CMake,
> > 
> > or
> > 
> > > downgrading CMake back to an older version that is compatible with Ninja
> > > being built without re2c being available.
> > > 
> > > Qt 6 CI will be globally disabled in 2 weeks time if this remains
> > 
> > unfixed,
> > 
> > > as dependencies move quickly and I'm not in favour of retaining parts of
> > > the CI system which cannot be rebuilt.
> > 
> > This is now done, don't be like me and waste your time trying to figure
> > out why
> > it works when it's not supposed to work.
> 
> Please note that the underlying reason for Ninja failing to compile within
> the CI Image build environment was never found, 

Are you sure about that?

Isn't 
https://invent.kde.org/packaging/craft/-/commit/49670cd2772e352df64749dd59d3a0437bf09d26
 the fix?

Cheers,
  Albert

> however it was worked
> around by using a pre-built version from Craft's cache.
> At some point in time in the not too distant future we will begin building
> Craft caches within our Docker images as well, which may expose this
> problem again.
> 
> For now though the immediate issue is resolved though yes.
> 
> > Cheers,
> > 
> >   Albert
> 
> Cheers,
> Ben
> 
> > > Regards,
> > > Ben
> > > 
> > > On Sat, Nov 19, 2022 at 7:55 AM Ben Cooksley  wrote:
> > > > Hi all,
> > > > 
> > > > Recently Sysadmin received a series of requests to rebuild the Docker
> > > > images used to support KDE CI services on invent.kde.org.
> > > > 
> > > > Unfortunately one of these rebuilds has exposed a bug of unknown
> > > > origin
> > > > (as it fails on our side but by all accounts works elsewhere) where
> > 
> > Craft
> > 
> > > > is unable to compile Ninja (with the compilation dying due to a
> > 
> > Makefile
> > 
> > > > syntax error that looks like a CMake bug).
> > > > 
> > > > The failure log can be found at
> > > > https://invent.kde.org/sysadmin/ci-images/-/jobs/601722
> > > > 
> > > > Subsequent to this we have also received a request to rebuild our
> > > > Linux
> > > > images to allow for Grantlee 5.3 to be used.
> > > > 
> > > > Given how development is conducted within some projects that make
> > > > heavy
> > > > use of Grantlee, and how some of that technology is used across
> > 
> > multiple
> > 
> > > > platforms it would be harmful to the wider CI system and KDE Community
> > 
> > to
> > 
> > > > allow for Grantlee 5.3 to become available on any of our platforms.
> > > > 
> > > > I'm therefore imposing a change freeze on all KDE CI Docker images
> > 
> > until
> > 
> > > > the issue with Craft/Ninja/CMake is resolved.
> > > > 
> > > > Should any project have prematurely adopted a mandatory dependency on
> > > > Grantlee 5.3 then as they have failed to follow the correct change
> > 
> > process
> > 
> > > > as documented on our wikis that change is deemed to be outside policy
> > 
> > and
> > 
> > > > should be reverted immediately.
> > > > 
> > > > Regards,
> > > > Ben Cooksley
> > > > KDE Sysadmin






Re: Docker CI Image Change Freeze

2022-12-11 Thread Albert Astals Cid
El dimarts, 29 de novembre de 2022, a les 10:15:33 (CET), Ben Cooksley va 
escriure:
> Hi all,
> 
> As an update to this, sufficient changes have been made within Craft that
> it is now possible to build Qt 5 images so i'm releasing the freeze for
> those images.
> Qt 6 remains broken, and therefore remains frozen at this time (see
> https://invent.kde.org/sysadmin/ci-images/-/jobs/619808)
> 
> As mentioned previously, I believe this to be a CMake bug given the lack of
> change in Ninja.
> 
> At this stage I would suggest investigation be focused on either upgrading
> to a newer version of Ninja that can handle the newer version of CMake, or
> downgrading CMake back to an older version that is compatible with Ninja
> being built without re2c being available.
> 
> Qt 6 CI will be globally disabled in 2 weeks time if this remains unfixed,
> as dependencies move quickly and I'm not in favour of retaining parts of
> the CI system which cannot be rebuilt.

This is now done, don't be like me and waste your time trying to figure out why 
it works when it's not supposed to work.

Cheers,
  Albert

> 
> Regards,
> Ben
> 
> On Sat, Nov 19, 2022 at 7:55 AM Ben Cooksley  wrote:
> > Hi all,
> > 
> > Recently Sysadmin received a series of requests to rebuild the Docker
> > images used to support KDE CI services on invent.kde.org.
> > 
> > Unfortunately one of these rebuilds has exposed a bug of unknown origin
> > (as it fails on our side but by all accounts works elsewhere) where Craft
> > is unable to compile Ninja (with the compilation dying due to a Makefile
> > syntax error that looks like a CMake bug).
> > 
> > The failure log can be found at
> > https://invent.kde.org/sysadmin/ci-images/-/jobs/601722
> > 
> > Subsequent to this we have also received a request to rebuild our Linux
> > images to allow for Grantlee 5.3 to be used.
> > 
> > Given how development is conducted within some projects that make heavy
> > use of Grantlee, and how some of that technology is used across multiple
> > platforms it would be harmful to the wider CI system and KDE Community to
> > allow for Grantlee 5.3 to become available on any of our platforms.
> > 
> > I'm therefore imposing a change freeze on all KDE CI Docker images until
> > the issue with Craft/Ninja/CMake is resolved.
> > 
> > Should any project have prematurely adopted a mandatory dependency on
> > Grantlee 5.3 then as they have failed to follow the correct change process
> > as documented on our wikis that change is deemed to be outside policy and
> > should be reverted immediately.
> > 
> > Regards,
> > Ben Cooksley
> > KDE Sysadmin






Re: New repo in kdereview: KWeather

2022-12-01 Thread Albert Astals Cid
El dimecres, 30 de novembre de 2022, a les 15:58:29 (CET), Devin va escriure:
> Should be fixed now, I removed the dialog flag on the window.

Confirmed :)

> 
> Thanks,
> Devin
> 
> On Tue, Nov 29, 2022 at 10:33 PM Nate Graham  wrote:
> > This happens on X11 when using QtDialogs for reasons I don't understand.
> > See
> > https://invent.kde.org/frameworks/knewstuff/-/commit/ea19fa6e824650f3257e8
> > 047d6f90e01899b2e03.
> > 
> > Nate
> > 
> > On 11/29/22 16:48, Devin wrote:
> > > Hmm, I have no idea why the behaviour is different for you, but I get:
> > > https://i.imgur.com/CvGD8HS.png
> > > 
> > > If you removed the "Qt.Dialog" flag here:
> > > https://invent.kde.org/plasma-mobile/kweather/-/blob/master/src/qml/sett
> > > ings/SettingsWindow.qml#L14, does it still have the issue?
> > > 
> > > Thanks,
> > > Devin
> > > 
> > > On Tue, Nov 29, 2022 at 6:44 PM Albert Astals Cid  wrote:
> > >> El dimecres, 30 de novembre de 2022, a les 0:33:39 (CET), Devin va 
escriure:
> > >>>> Ok, now that i updated kirigami-addons, the Settings dialog shows,
> > >>>> but
> > >>>> it's uncloseable, any idea why that would be?
> > >>> 
> > >>> That's very strange, it should be showing up as a regular window, does
> > >>> pressing the close button on the window decoration not work?
> > >> 
> > >> Which close button?
> > >> 
> > >> https://i.imgur.com/2jX2yUS.png
> > >> 
> > >> Cheers,
> > >> 
> > >>Albert
> > >>> 
> > >>> Thanks,
> > >>> Devin
> > >>> 
> > >>> On Tue, Nov 29, 2022 at 6:23 PM Albert Astals Cid  
wrote:
> > >>>> El dilluns, 28 de novembre de 2022, a les 17:15:57 (CET), Albert
> > >>>> Astals
> > >>>> Cid va>
> > >>>> 
> > >>>> escriure:
> > >>>>> El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va
> > >>>> 
> > >>>> escriure:
> > >>>>>>> There's potentially overlapping text.
> > >>>>>> 
> > >>>>>> I have fixed text wrapping in the weather delegates so it should
> > >>>>>> wrap
> > >>>>>> properly now.
> > >>>>>> 
> > >>>>>>> Here settingsModel.forecastStyle is not i18n'ed
> > >>>>>> 
> > >>>>>> I have fixed it now (it converts to i18n'd dedicated strings in the
> > >>>>>> QML)
> > >>>>> 
> > >>>>> FWIW i can't open settings anymore
> > >>>>> 
> > >>>>> qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> > >>>>> unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot
> > >>>>> assign
> > >>>>> to
> > >>>>> non-existent property "onActivated" qrc:/qml/main.qml:136:
> > >>>>> TypeError:
> > >>>>> Cannot call method 'close' of null
> > >>>> 
> > >>>> Ok, now that i updated kirigami-addons, the Settings dialog shows,
> > >>>> but
> > >>>> it's
> > >>>> uncloseable, any idea why that would be?
> > >>>> 
> > >>>> Cheers,
> > >>>> 
> > >>>>Albert
> > >>>>> 
> > >>>>> Cheers,
> > >>>>> 
> > >>>>>Albert
> > >>>>>> 
> > >>>>>> On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid 
> > >> 
> > >> wrote:
> > >>>>>>> El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va
> > >>>> 
> > >>>> escriure:
> > >>>>>>>> Hi everyone,
> > >>>>>>>> 
> > >>>>>>>> I would like to put kweather through kdereview:
> > >>>>>>>> 
> > >>>>>>>> https://invent.kde.org/plasma-mobile/kweather
> > >>>>>>>> 
> > >>>>>>>> KWeather is an application that can give simple weather
> > >>>>>>>> information
> > >>>>>>>> for different weather locations. Please note that KWeatherCore
> > >>>>>>>> (the
> > >>>>>>>> library the app depends on) has already passed kdereview.
> > >>>>>>> 
> > >>>>>>> There's potentially overlapping text.
> > >>>>>>> 
> > >>>>>>> See https://i.imgur.com/BrOgi3A.png
> > >>>>>>> 
> > >>>>>>> The settings show untranslateable text, i.e.
> > >>>>>>> 
> > >>>>>>> MobileForm.FormComboBoxDelegate {
> > >>>>>>> 
> > >>>>>>>  id: forecastStyleDropdown
> > >>>>>>>  text: i18n("Forecast Style")
> > >>>>>>>  currentValue: settingsModel.forecastStyle
> > >>>>>>> 
> > >>>>>>> Here settingsModel.forecastStyle is not i18n'ed
> > >>>>>>> 
> > >>>>>>> Not KWeather's fault but i just realized that Kirigami's about
> > >>>>>>> page
> > >>>>>>> doesn't
> > >>>>>>> properly credit translators :/
> > >>>>>>> 
> > >>>>>>> Cheers,
> > >>>>>>> 
> > >>>>>>>Albert
> > >>>>>>>> 
> > >>>>>>>> Thanks,
> > >>>>>>>> Devin






Re: New repo in kdereview: KWeather

2022-11-29 Thread Albert Astals Cid
El dimecres, 30 de novembre de 2022, a les 0:33:39 (CET), Devin va escriure:
> > Ok, now that i updated kirigami-addons, the Settings dialog shows, but
> > it's uncloseable, any idea why that would be?
> That's very strange, it should be showing up as a regular window, does
> pressing the close button on the window decoration not work?

Which close button?

https://i.imgur.com/2jX2yUS.png

Cheers,
  Albert

> 
> Thanks,
> Devin
> 
> On Tue, Nov 29, 2022 at 6:23 PM Albert Astals Cid  wrote:
> > El dilluns, 28 de novembre de 2022, a les 17:15:57 (CET), Albert Astals
> > Cid va> 
> > escriure:
> > > El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va
> > 
> > escriure:
> > > > > There's potentially overlapping text.
> > > > 
> > > > I have fixed text wrapping in the weather delegates so it should wrap
> > > > properly now.
> > > > 
> > > > > Here settingsModel.forecastStyle is not i18n'ed
> > > > 
> > > > I have fixed it now (it converts to i18n'd dedicated strings in the
> > > > QML)
> > > 
> > > FWIW i can't open settings anymore
> > > 
> > > qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> > > unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign
> > > to
> > > non-existent property "onActivated" qrc:/qml/main.qml:136: TypeError:
> > > Cannot call method 'close' of null
> > 
> > Ok, now that i updated kirigami-addons, the Settings dialog shows, but
> > it's
> > uncloseable, any idea why that would be?
> > 
> > Cheers,
> > 
> >   Albert
> >   
> > > Cheers,
> > > 
> > >   Albert
> > >   
> > > > On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid  
wrote:
> > > > > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va
> > 
> > escriure:
> > > > > > Hi everyone,
> > > > > > 
> > > > > > I would like to put kweather through kdereview:
> > > > > > 
> > > > > > https://invent.kde.org/plasma-mobile/kweather
> > > > > > 
> > > > > > KWeather is an application that can give simple weather
> > > > > > information
> > > > > > for different weather locations. Please note that KWeatherCore
> > > > > > (the
> > > > > > library the app depends on) has already passed kdereview.
> > > > > 
> > > > > There's potentially overlapping text.
> > > > > 
> > > > > See https://i.imgur.com/BrOgi3A.png
> > > > > 
> > > > > The settings show untranslateable text, i.e.
> > > > > 
> > > > > MobileForm.FormComboBoxDelegate {
> > > > > 
> > > > > id: forecastStyleDropdown
> > > > > text: i18n("Forecast Style")
> > > > > currentValue: settingsModel.forecastStyle
> > > > > 
> > > > > Here settingsModel.forecastStyle is not i18n'ed
> > > > > 
> > > > > Not KWeather's fault but i just realized that Kirigami's about page
> > > > > doesn't
> > > > > properly credit translators :/
> > > > > 
> > > > > Cheers,
> > > > > 
> > > > >   Albert
> > > > >   
> > > > > > Thanks,
> > > > > > Devin






Re: New repo in kdereview: KWeather

2022-11-29 Thread Albert Astals Cid
El dilluns, 28 de novembre de 2022, a les 17:15:57 (CET), Albert Astals Cid va 
escriure:
> El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va 
escriure:
> > > There's potentially overlapping text.
> > 
> > I have fixed text wrapping in the weather delegates so it should wrap
> > properly now.
> > 
> > > Here settingsModel.forecastStyle is not i18n'ed
> > 
> > I have fixed it now (it converts to i18n'd dedicated strings in the QML)
> 
> FWIW i can't open settings anymore
> 
> qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign to
> non-existent property "onActivated" qrc:/qml/main.qml:136: TypeError:
> Cannot call method 'close' of null

Ok, now that i updated kirigami-addons, the Settings dialog shows, but it's 
uncloseable, any idea why that would be?

Cheers,
  Albert

> 
> Cheers,
>   Albert
> 
> > On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid  wrote:
> > > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va 
escriure:
> > > > Hi everyone,
> > > > 
> > > > I would like to put kweather through kdereview:
> > > > 
> > > > https://invent.kde.org/plasma-mobile/kweather
> > > > 
> > > > KWeather is an application that can give simple weather information
> > > > for different weather locations. Please note that KWeatherCore (the
> > > > library the app depends on) has already passed kdereview.
> > > 
> > > There's potentially overlapping text.
> > > 
> > > See https://i.imgur.com/BrOgi3A.png
> > > 
> > > The settings show untranslateable text, i.e.
> > > 
> > > MobileForm.FormComboBoxDelegate {
> > > 
> > > id: forecastStyleDropdown
> > > text: i18n("Forecast Style")
> > > currentValue: settingsModel.forecastStyle
> > > 
> > > Here settingsModel.forecastStyle is not i18n'ed
> > > 
> > > Not KWeather's fault but i just realized that Kirigami's about page
> > > doesn't
> > > properly credit translators :/
> > > 
> > > Cheers,
> > > 
> > >   Albert
> > >   
> > > > Thanks,
> > > > Devin






Re: Tokodon in KDE Review

2022-11-28 Thread Albert Astals Cid
El dijous, 24 de novembre de 2022, a les 15:00:44 (CET), Carl Schwan va 
escriure:
> Hello everyone,
> 
> I would like to put Tokodon in KDE Review. Tokodon is mastodon client, I
> have been working for some time already. I already did some unstable
> release and I would like to soon release the first stable release. The
> source code is available here: https://invent.kde.org/network/tokodon/
> 
> It is still missing a lot of Mastodon features (lists, support for custom
> emojies, displaying and sending polls, ...) but the current feature sets
> make it usable already. And I am slowly adding more features.
> 
> Cheers,
> Carl
> 
> PS: if someone has insight about this crash deep in the QML engine, I would
> be welcome some help https://bugs.kde.org/show_bug.cgi?id=461882

Crashes 

$ tokodon 
Loading any accounts from settings.
qrc:/content/ui/main.qml:74: TypeError: Cannot read property 'instanceName' of 
null
Violació de segment (s'ha bolcat la memòria)

Backtrace https://pst.klgrth.io/paste/s87zy

You can see that if you add 
onCheckedChanged: console.log("BLO", checked);
to notificationAction it will switch infinitely from true to false and and 
crash.

Cheers,
  Albert




Re: New repo in kdereview: KWeather

2022-11-28 Thread Albert Astals Cid
El dilluns, 28 de novembre de 2022, a les 20:18:14 (CET), Devin va escriure:
> > Depending on an unreleased version of a dependency is not acceptable if
> > you
> > want to release your application (which it is my understanding of what
> > you're aiming given you're asking us to review it).
> 
> It's a bit of a strange situation since KWeather had already been
> released along with Kirigami Addons in Plasma Mobile Gear for a bit
> more than a year.
> 
> Kirigami Addons had a breaking change for this release, which is why
> KWeather had to update its API usage (they are released at the same
> time).

You've been told that you need to increase your minimum dependency in another 
email, i'll do it again.

Increase your required version of kirigami addons so that it matches reality.

Cheers,
  Albert

> 
> Thanks,
> Devin
> 
> On Mon, Nov 28, 2022 at 12:23 PM Albert Astals Cid  wrote:
> > El dilluns, 28 de novembre de 2022, a les 17:53:59 (CET), Devin va 
escriure:
> > > > FWIW i can't open settings anymore
> > > 
> > > Make sure you have the latest kirigami-addons built (from master).
> > 
> > Depending on an unreleased version of a dependency is not acceptable if
> > you
> > want to release your application (which it is my understanding of what
> > you're aiming given you're asking us to review it).
> > 
> > Cheers,
> > 
> >   Albert
> >   
> > > Thanks,
> > > Devin
> > > 
> > > On Mon, Nov 28, 2022 at 11:15 AM Albert Astals Cid  
wrote:
> > > > El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va
> > 
> > escriure:
> > > > > > There's potentially overlapping text.
> > > > > 
> > > > > I have fixed text wrapping in the weather delegates so it should
> > > > > wrap
> > > > > properly now.
> > > > > 
> > > > > > Here settingsModel.forecastStyle is not i18n'ed
> > > > > 
> > > > > I have fixed it now (it converts to i18n'd dedicated strings in the
> > > > > QML)
> > > > 
> > > > FWIW i can't open settings anymore
> > > > 
> > > > qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> > > > unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot
> > > > assign
> > > > to non-existent property "onActivated" qrc:/qml/main.qml:136:
> > > > TypeError:
> > > > Cannot call method 'close' of null
> > > > 
> > > > Cheers,
> > > > 
> > > >   Albert
> > > >   
> > > > > On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid  
wrote:
> > > > > > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va
> > 
> > escriure:
> > > > > > > Hi everyone,
> > > > > > > 
> > > > > > > I would like to put kweather through kdereview:
> > > > > > > 
> > > > > > > https://invent.kde.org/plasma-mobile/kweather
> > > > > > > 
> > > > > > > KWeather is an application that can give simple weather
> > > > > > > information
> > > > > > > for different weather locations. Please note that KWeatherCore
> > > > > > > (the
> > > > > > > library the app depends on) has already passed kdereview.
> > > > > > 
> > > > > > There's potentially overlapping text.
> > > > > > 
> > > > > > See https://i.imgur.com/BrOgi3A.png
> > > > > > 
> > > > > > The settings show untranslateable text, i.e.
> > > > > > 
> > > > > > MobileForm.FormComboBoxDelegate {
> > > > > > 
> > > > > > id: forecastStyleDropdown
> > > > > > text: i18n("Forecast Style")
> > > > > > currentValue: settingsModel.forecastStyle
> > > > > > 
> > > > > > Here settingsModel.forecastStyle is not i18n'ed
> > > > > > 
> > > > > > Not KWeather's fault but i just realized that Kirigami's about
> > > > > > page
> > > > > > doesn't
> > > > > > properly credit translators :/
> > > > > > 
> > > > > > Cheers,
> > > > > > 
> > > > > >   Albert
> > > > > >   
> > > > > > > Thanks,
> > > > > > > Devin






Re: New repo in kdereview: KWeather

2022-11-28 Thread Albert Astals Cid
El dilluns, 28 de novembre de 2022, a les 17:53:59 (CET), Devin va escriure:
> > FWIW i can't open settings anymore
> 
> Make sure you have the latest kirigami-addons built (from master).

Depending on an unreleased version of a dependency is not acceptable if you 
want to release your application (which it is my understanding of what you're 
aiming given you're asking us to review it).

Cheers,
  Albert

> 
> Thanks,
> Devin
> 
> On Mon, Nov 28, 2022 at 11:15 AM Albert Astals Cid  wrote:
> > El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va 
escriure:
> > > > There's potentially overlapping text.
> > > 
> > > I have fixed text wrapping in the weather delegates so it should wrap
> > > properly now.
> > > 
> > > > Here settingsModel.forecastStyle is not i18n'ed
> > > 
> > > I have fixed it now (it converts to i18n'd dedicated strings in the QML)
> > 
> > FWIW i can't open settings anymore
> > 
> > qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> > unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign
> > to non-existent property "onActivated" qrc:/qml/main.qml:136: TypeError:
> > Cannot call method 'close' of null
> > 
> > Cheers,
> > 
> >   Albert
> >   
> > > On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid  wrote:
> > > > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va 
escriure:
> > > > > Hi everyone,
> > > > > 
> > > > > I would like to put kweather through kdereview:
> > > > > 
> > > > > https://invent.kde.org/plasma-mobile/kweather
> > > > > 
> > > > > KWeather is an application that can give simple weather information
> > > > > for different weather locations. Please note that KWeatherCore (the
> > > > > library the app depends on) has already passed kdereview.
> > > > 
> > > > There's potentially overlapping text.
> > > > 
> > > > See https://i.imgur.com/BrOgi3A.png
> > > > 
> > > > The settings show untranslateable text, i.e.
> > > > 
> > > > MobileForm.FormComboBoxDelegate {
> > > > 
> > > > id: forecastStyleDropdown
> > > > text: i18n("Forecast Style")
> > > > currentValue: settingsModel.forecastStyle
> > > > 
> > > > Here settingsModel.forecastStyle is not i18n'ed
> > > > 
> > > > Not KWeather's fault but i just realized that Kirigami's about page
> > > > doesn't
> > > > properly credit translators :/
> > > > 
> > > > Cheers,
> > > > 
> > > >   Albert
> > > >   
> > > > > Thanks,
> > > > > Devin






Re: New repo in kdereview: KWeather

2022-11-28 Thread Albert Astals Cid
El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va escriure:
> > There's potentially overlapping text.
> 
> I have fixed text wrapping in the weather delegates so it should wrap
> properly now.
> 
> > Here settingsModel.forecastStyle is not i18n'ed
> 
> I have fixed it now (it converts to i18n'd dedicated strings in the QML)

FWIW i can't open settings anymore

qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent unavailable
qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign to non-existent 
property "onActivated"
qrc:/qml/main.qml:136: TypeError: Cannot call method 'close' of null

Cheers,
  Albert

> 
> On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid  wrote:
> > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va escriure:
> > > Hi everyone,
> > > 
> > > I would like to put kweather through kdereview:
> > > 
> > > https://invent.kde.org/plasma-mobile/kweather
> > > 
> > > KWeather is an application that can give simple weather information
> > > for different weather locations. Please note that KWeatherCore (the
> > > library the app depends on) has already passed kdereview.
> > 
> > There's potentially overlapping text.
> > 
> > See https://i.imgur.com/BrOgi3A.png
> > 
> > The settings show untranslateable text, i.e.
> > 
> > MobileForm.FormComboBoxDelegate {
> > 
> > id: forecastStyleDropdown
> > text: i18n("Forecast Style")
> > currentValue: settingsModel.forecastStyle
> > 
> > Here settingsModel.forecastStyle is not i18n'ed
> > 
> > Not KWeather's fault but i just realized that Kirigami's about page
> > doesn't
> > properly credit translators :/
> > 
> > Cheers,
> > 
> >   Albert
> >   
> > > Thanks,
> > > Devin






Re: New repo in kdereview: KWeather

2022-11-14 Thread Albert Astals Cid
El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va escriure:
> Hi everyone,
> 
> I would like to put kweather through kdereview:
> 
> https://invent.kde.org/plasma-mobile/kweather
> 
> KWeather is an application that can give simple weather information
> for different weather locations. Please note that KWeatherCore (the
> library the app depends on) has already passed kdereview.

There's potentially overlapping text.

See https://i.imgur.com/BrOgi3A.png

The settings show untranslateable text, i.e.

MobileForm.FormComboBoxDelegate {
id: forecastStyleDropdown
text: i18n("Forecast Style")
currentValue: settingsModel.forecastStyle

Here settingsModel.forecastStyle is not i18n'ed

Not KWeather's fault but i just realized that Kirigami's about page doesn't 
properly credit translators :/

Cheers,
  Albert

> 
> Thanks,
> Devin






Re: ghostwriter is ready for your review

2022-11-01 Thread Albert Astals Cid
El dijous, 13 d’octubre de 2022, a les 8:52:26 (CET), Megan va escriure:
> Hello everyone,
> 
> The /ghostwriter/ Markdown editor has finally hatched from its
> incubation and is ready for you to review at your convenience.
> 
> Project repo: https://invent.kde.org/office/ghostwriter
> 
> Project website: https://ghostwriter.kde.org
> 
> Note: ghostwriter currently uses QtLinguist for translations. However, I
> plan to transition it to ki18n as soon as I can.  Any help you can
> provide would be appreciated, of course!

I've just realized the man page is not translatable.

See https://invent.kde.org/multimedia/dragon/-/tree/master/doc how to make a 
manpage using docbook so its translatable.

Cheers,
  Albert

> 
> Thanks so much!
> 
> Megan






Re: New repo in kdereview: KRecorder

2022-10-21 Thread Albert Astals Cid
El divendres, 21 d’octubre de 2022, a les 23:55:28 (CEST), Devin va escriure:
> > make install doesn't install any icon for me with the current master.
> 
> I just checked and indeed, the method I changed to using
> ecm_install_icons doesn't seem to have the behaviour I thought it did.
> I hadn't verified it properly because the icon was already
> preinstalled for me.
> 
> I reverted to the prior commit which installed the icon fine (it
> should be installing to
> /usr/share/icons/hicolor/scalable/apps/krecorder.svg), does this not
> work for you on X11? I double checked and the application icon shows
> for me.

https://invent.kde.org/plasma-mobile/krecorder/-/merge_requests/17

Makes it work for me (when starting from the terminal)

Cheers,
  Albert

> 
> Thanks,
> Devin
> 
> On Fri, Oct 21, 2022 at 5:08 PM Albert Astals Cid  wrote:
> > El divendres, 21 d’octubre de 2022, a les 23:00:46 (CEST), Devin va 
escriure:
> > > > The app doesn't have an icon when run in X11
> > > 
> > > Hmm, the location the icon installed to might be non-standard. I think
> > > I've fixed it on master now by copying the way other KDE apps install
> > > the icon.
> > 
> > make install doesn't install any icon for me with the current master.
> > 
> > Cheers,
> > 
> >   Albert






Re: New repo in kdereview: KRecorder

2022-10-21 Thread Albert Astals Cid
El divendres, 21 d’octubre de 2022, a les 23:00:46 (CEST), Devin va escriure:
> > The app doesn't have an icon when run in X11
> 
> Hmm, the location the icon installed to might be non-standard. I think
> I've fixed it on master now by copying the way other KDE apps install
> the icon.

make install doesn't install any icon for me with the current master.

Cheers,
  Albert




Re: New repo in kdereview: QMLKonsole

2022-10-21 Thread Albert Astals Cid
El divendres, 21 d’octubre de 2022, a les 4:28:13 (CEST), Devin va escriure:
> Hi everyone,
> 
> I would like to put qmlkonsole through kdereview:
> 
> https://invent.kde.org/plasma-mobile/qmlkonsole
> 
> QMLKonsole is an Qt Quick based terminal emulator application for Plasma
> Mobile. 

qrc:/SavedCommandsDialog.qml:44:9: QML PlaceholderMessage: Cannot specify 
left, right, and horizontalCenter anchors at the same time.

When run on X11 doesn't seem to have an icon.

How happy are with the name given it has "nothing" to do with konsole?

> It depends on qmltermwidget:
> https://github.com/Swordfish90/qmltermwidget 

This thing has had almost 200 commits since the last release 4 years ago, that 
ain't great. Do we have any contact with the person to get them to release a 
bit more often?

It doesn't even have a issue tracker where i can report that the need to init 
TerminalDisplay::m_session to nullptr :/

Cheers,
  Albert

> Thanks,
> Devin






Re: New repo in kdereview: KRecorder

2022-10-21 Thread Albert Astals Cid
El divendres, 21 d’octubre de 2022, a les 4:25:44 (CEST), Devin va escriure:
> Hi everyone,
> 
> There will be some Plasma Mobile Gear applications going through here since
> it seems some did not make it through kdereview. Our eventual goal is to
> hopefully move them to the KDE Gear release :)
> 
> I would like to put krecorder through kdereview:
> 
> https://invent.kde.org/plasma-mobile/krecorder
> 
> KRecorder is an audio recording application for Plasma Mobile and Desktop.

* open krecoder
* Do a recording
* gets saved as /home/tsdgeos/clip_0001.mov
* Open settings
* Close settings
* Do a recording
* gets saved as /home/tsdgeos/clip_0002.ogg

Why the mov vs ogg dance?

The app doesn't have an icon when run in X11

The model was kind-of-potentially an infinite tree, fixed with other small 
tweaks at https://invent.kde.org/plasma-mobile/krecorder/-/merge_requests/15

Cheers,
  Albert

> 
> Thanks,
> Devin






Re: Plasma Welcome Center on KDEReview

2022-10-19 Thread Albert Astals Cid
El dimecres, 19 d’octubre de 2022, a les 5:22:09 (CEST), Nate Graham va 
escriure:
> Ping; if there are no objections I'd consider it approved.

I can't still see the toolbar :/

Albert 

> 
> Nate
> 
> On 10/6/22 02:13, Nate Graham wrote:
> > There have been no additional comments or objections for two weeks; can
> > everything brought up in this thread be considered resolved now?
> > 
> > Nate
> > 
> > On 9/23/22 23:34, Nate Graham wrote:
> >> Thanks for your help, everyone. Fixed now with
> >> https://invent.kde.org/plasma/plasma-welcome/-/merge_requests/12.
> >> 
> >> Nate
> >> 
> >> On 9/18/22 11:03, Harald Sitter wrote:
> >>> Not all code is licensing policy compliant (e.g. appdata is missing
> >>> spdx tags). I'd suggest enabling the reuse linter pipeline.
> >>> 
> >>> On Fri, Sep 16, 2022 at 6:00 PM Nate Graham  wrote:
>  Hello folks!
>  
>  I've been working with Aleix on an onboarding wizard for Plasma, based
>  on work originally started by Felipe Kinoshita last year.
>  
>  You can see some screenshots at
>  https://invent.kde.org/websites/product-screenshots/-/commit/e300be66e6
>  2263c63d8a85ba391bbcc1de691148.
>  
>  I'd like to target Plasma 5.27 for release and am submitting it for
>  KDEReview. The repo is at https://invent.kde.org/plasma/welcome-app.
>  
>  
>  Nate






Re: ghostwriter is ready for your review

2022-10-17 Thread Albert Astals Cid
El diumenge, 16 d’octubre de 2022, a les 5:05:40 (CEST), Megan va escriure:
> Hi Albert,
> 
> > If you're going to change something as core to an app as the i18n system I
> > wouldn't say we're done for review stage yet. Or maybe you don't need to
> > change it?
> 
> Carl said it should be enough to pass review with a Messages.sh and
> including ECMPoQmTools in the CMakeLists.txt file.  Also, he noted that
> other apps are still using QtLinguist, such as Analitza and Stopmotion.

Using Qt for translations is fine acceptable.

Doing a "major" rewrite after review kind of invalidates the review, and 
that's why I say that if you're going to rewrite it we should do the review 
later.

> 
> > I have lots of actions with the ¿same icon? is that supposed to be like
> > that or we just don't have those in
> > breeze?https://i.imgur.com/m2Ytkcc.png
> It's definitely not supposed to look like that.  I tried a fresh install
> on my machine (removing and rebuilding from scratch) but could not
> replicate the issue.  It's supposed to be using Font Awesome's font
> glyphs for the icons, since they are easily styled along with the normal
> text in QSS/CSS.  I also double checked that I don't have Font Awesome
> installed as a font.  Weird.
> 
> > You have both a CMakeLists and a qmake pro. I'd suggest to remove one,
> > specially the pro one, maintaining 2 build systems will only bring you
> > sadness.
> 
> Agreed.  I had forgotten to remove it. It's gone now.  Thanks!
> 
> > You have 4 libs in the 3rdparty folder, is there any chance to use actual
> > dependencies and not copied code?
> 
> 1. Unfortunately, some of the dependencies aren't in every GNU/Linux
> distribution.
> 2. It is easier for doing Windows and MacOS builds to have the
> dependencies bundled with the app code.
> 3. To protect against sudden API changes across distros, it's best to
> freeze the versions of the dependencies needed by keeping them bundled. 
> This way I can upgrade them when I'm prepared to rather than as an
> emergency fix.
> 
> > Using KXmlGui would maybe make sense?
> 
> Yes, I may very well do that in the future.  It looks very useful.
> 
> > When running i get
> > 
> >Command "pandoc" is not available.
> >Command "multimarkdown" is not available.
> >Command "cmark" is not available.
> > 
> > on the command line, "normal" users will start the app via the menu and
> > won't get to see that.
> 
> That's largely for my own use so I can extract better information from
> people reporting issues in the bug tracker.  I would like to move this
> to the About dialog in the future.  I agree that would be better.  (But
> can I do that later and still pass review?)
> 
> > The theme choose dialog only has "Close", i'm guess that "ok" would make
> > more sense given that it sets the new theme when pressing it? I may be
> > wrong there, so feel free to disagree (well here and with everything :D)
> 
> Yes, "OK" does give the sense that your changes will take place. On the
> other hand, it might also convey to the user that closing the window
> from the upper corner will cancel the changes.  I wouldn't want to give
> that false impression.  But if you still feel that "OK" is better, I
> will change it there and in the Preferences dialog.  :)
> 
> > For some reason it thinks my language is the valencian variant of catalan,
> > when it should be non-variant catalan one.
> 
> I will look into this and come up with a fix.  :)
> 
> > There seems to be a huge memory leak regarding sonnet and CmarkGfm usage,
> > see what valgrind tells mehttps://pst.klgrth.io/paste/wc6vn
> 
> For the CmarkGfm memory leak report, I wonder if the creation and return
> of a new MarkdownAST is the cause?  This class is handed off to the
> MarkdownDocument class, which handles deleting it when either a new one
> is set or its destructor is called.  Maybe valgrind isn't smart enough
> to notice that?  

VERY unlikely that you found a valgrind bug.

https://invent.kde.org/office/ghostwriter/-/merge_requests/8

Cheers,
  Albert

P.S: The amount of this-> in the code is very much non customary in C++ code. 
Not wrong but feels itchy :D

> I really don't know.  Do you have any insight into this?
> 
> As for the Sonnet usage, that almost looks like Hunspell is the cause?
> 
> I ran valgrind myself, and I see Sonnet/Hunspell issue. Strangely I
> don't see the CmarkGfm one. (But I see plenty for the AppSettings
> singleton class

Re: kio-admin in kdereview

2022-10-17 Thread Albert Astals Cid
El dimecres, 12 d’octubre de 2022, a les 10:40:45 (CEST), Harald Sitter va 
escriure:
> Hola
> 
> kio-admin implements an admin worker that gives root level access to
> the file system
> 
> https://invent.kde.org/system/kio-admin

Can you please have a look at the MR I created?

Cheers,
  Albert

> 
> HS






Re: ghostwriter is ready for your review

2022-10-15 Thread Albert Astals Cid
El dijous, 13 d’octubre de 2022, a les 8:52:26 (CEST), Megan va escriure:
> Hello everyone,
> 
> The /ghostwriter/ Markdown editor has finally hatched from its
> incubation and is ready for you to review at your convenience.
> 
> Project repo: https://invent.kde.org/office/ghostwriter
> 
> Project website: https://ghostwriter.kde.org
> 
> Note: ghostwriter currently uses QtLinguist for translations. However, I
> plan to transition it to ki18n as soon as I can.  Any help you can
> provide would be appreciated, of course!

If you're going to change something as core to an app as the i18n system I 
wouldn't say we're done for review stage yet. Or maybe you don't need to 
change it?

anyhow here's my quick look

I have lots of actions with the ¿same icon? is that supposed to be like that 
or we just don't have those in breeze? https://i.imgur.com/m2Ytkcc.png

You have both a CMakeLists and a qmake pro. I'd suggest to remove one, 
specially the pro one, maintaining 2 build systems will only bring you 
sadness.

You have 4 libs in the 3rdparty folder, is there any chance to use actual 
dependencies and not copied code?

Using KXmlGui would maybe make sense?

When running i get
  Command "pandoc" is not available.
  Command "multimarkdown" is not available.
  Command "cmark" is not available.
on the command line, "normal" users will start the app via the menu and won't 
get to see that.


The theme choose dialog only has "Close", i'm guess that "ok" would make more 
sense given that it sets the new theme when pressing it? I may be wrong there, 
so feel free to disagree (well here and with everything :D)

For some reason it thinks my language is the valencian variant of catalan, 
when it should be non-variant catalan one.

There seems to be a huge memory leak regarding sonnet and CmarkGfm usage, see 
what valgrind tells me https://pst.klgrth.io/paste/wc6vn

Cheers,
  Albert


> 
> Thanks so much!
> 
> Megan






Re: kio-admin in kdereview

2022-10-15 Thread Albert Astals Cid
El divendres, 14 d’octubre de 2022, a les 10:34:04 (CEST), Harald Sitter va 
escriure:
> On Thu, Oct 13, 2022 at 10:32 PM Albert Astals Cid  wrote:
> > El dijous, 13 d’octubre de 2022, a les 1:03:53 (CEST), Harald Sitter va
> > 
> > escriure:
> > > On Thu, Oct 13, 2022 at 12:46 AM Albert Astals Cid  
wrote:
> > > > Did I misunderstood the code? It looks like this run all of kio with
> > > > root
> > > > powers?
> > > 
> > > That is correct
> > 
> > That feels like a reasonably big no no with my security hat.
> > 
> > I'm relatively sure we have not audited all of KIO and it's dependencies
> > to be "running as root"-safe.
> 
> It is scary to be sure, but then the user has to opt into shooting in the
> foot.

How much of that opt in message mentions potential security issues?

> > What's the use case of this against the kauth support in file_unix.cpp ?
> 
> The latter doesn't exist :(

There is a great deal of code that does auth stuff, it's just preceded by a 

// temporarily disable privilege execution

Does anyone know what's the deal with that?

Because if the code is good we should enable it, and if the code is bad we 
should probably rip it off?

Cheers,
  Albert

> 
> HS






Re: kio-admin in kdereview

2022-10-13 Thread Albert Astals Cid
El dijous, 13 d’octubre de 2022, a les 1:03:53 (CEST), Harald Sitter va 
escriure:
> On Thu, Oct 13, 2022 at 12:46 AM Albert Astals Cid  wrote:
> > Did I misunderstood the code? It looks like this run all of kio with root
> > powers?
> 
> That is correct

That feels like a reasonably big no no with my security hat.

I'm relatively sure we have not audited all of KIO and it's dependencies to be 
"running as root"-safe.

What's the use case of this against the kauth support in file_unix.cpp ?

Cheers,
  Albert




Re: kio-admin in kdereview

2022-10-12 Thread Albert Astals Cid
El dimecres, 12 d’octubre de 2022, a les 10:40:45 (CEST), Harald Sitter va 
escriure:
> Hola
> 
> kio-admin implements an admin worker that gives root level access to
> the file system
> 
> https://invent.kde.org/system/kio-admin

qDebug() << "actions!!!";
qDebug() << "urly!!!" << url;
probably needs to go away or be a qCDebug?

Did I misunderstood the code? It looks like this run all of kio with root 
powers?

Cheers,
  Albert 

> 
> HS






Re: KJournald in KDE-Review

2022-10-09 Thread Albert Astals Cid
El diumenge, 9 d’octubre de 2022, a les 19:18:15 (CEST), Andreas Cord-Landwehr 
va escriure:
> Hi, after a few releases over the past year, I would like to get KJournald
> included in KDE application releases. This project is about providing both
> an QItemModel abstraction library for the C-style journald API and
> providing an efficient graphical browser for journald logs.
> 
> Sysadmins moved the repository to KDE Review today, you can find the source
> code here:
> 
> https://invent.kde.org/libraries/kjournald
> 
> (flatpack packaging is also available for easy trying it out)
> 
> Even though KJournald is currently contained in the "libraries" playground
> module, I would like to get it included in the "utilities" module after
> passing KDE Review. The reason is that at the moment I more focus on the
> application part and that is the most user-facing part. Having it in
> "utilities" thus will avoid confusion.
> 
> Looking forward for your review comments!

automoc complains a bit about some properties https://pst.klgrth.io/paste/kf9ta
If you're planning to use those from QML maybe a Q_INVOKABLE is better?

You need to use -DTRANSLATION_DOMAIN on your library (i understand it's 
supposed to be usable by third party apps)

valgrind is reporting a massive memory leak, you need to free the char* 
returned by sd_journal_get_cursor

Cheers,
  Albert

> 
> Best regards,
> Andreas






Re: KDEReview for kio-s3

2022-09-20 Thread Albert Astals Cid
El dimarts, 20 de setembre de 2022, a les 0:17:52 (CEST), Elvis Angelaccio va 
escriure:
> Hi all,
> I think kio-s3 [1] is now ready to go through the KDEReview process and
> get a first release.
> 
> dfaure and sitter did a first code review in the original MR against
> kio-extras [1]. The code hasn't changed much since then, mostly the port
> to the new WorkerBase interface.
> 
> Please have a look. Thanks :)

Small enough, i didn't find anything to mention :)

Cheers,
  Albert

> 
> P.S.
> You might notice that the gitlab CI is currently broken, I have already
> filed a sysadmin ticket trying to sort it out.
> 
> [1]: https://invent.kde.org/network/kio-s3
> [2]: https://invent.kde.org/network/kio-extras/-/merge_requests/35
> 
> 
> Cheers,
> Elvis






Re: Plasma Welcome Center on KDEReview

2022-09-18 Thread Albert Astals Cid
El dissabte, 17 de setembre de 2022, a les 17:18:18 (CEST), Nate Graham va 
escriure:
> On 9/17/22 02:16, Albert Astals Cid wrote:
> > Understanding that this is a standalone app and none of its contents are
> > reused to be shown in other apps a
> > 
> >KLocalizedString::setApplicationDomain("welcomecenter");
> > 
> > call in its main.cpp will do.
> > 
> > Sidenote, given the name of the app is plasma-welcome i'd really suggest
> > changing the catalog name to plasma-welcome too instead of welcomecenter.
> 
> Done.
> 
> > It's a kirigami app, sadly this means it has too many warnings
> > https://pst.klgrth.io/paste/ydbmz
> 
> A few of those are already fixed in git master. For the others, I've
> tried to clean this up a bit by fixing warnings where I can, or at least
> filing bug reports:
> - https://invent.kde.org/frameworks/kirigami/-/merge_requests/748
> - https://invent.kde.org/network/kaccounts-integration/-/merge_requests/42
> https://invent.kde.org/plasma/welcome-app/-/merge_requests/8
> - https://bugs.kde.org/show_bug.cgi?id=459283
> - https://bugs.kde.org/show_bug.cgi?id=459284
> 
> > When run on my computer i can't see any buttons on the top "toolbar" (they
> > are there since generally clicking where they should be gets the thing to
> > go next/ prev). What dependency version are you interested in knowing
> > that may be causing this?
> 
> This is weird. I don't understand how the toolbar can be invisible but
> interactive for you. I'm at a loss here. Does anyone else have any ideas?

https://invent.kde.org/plasma/welcome-app/-/merge_requests/9

> 
> 
> Nate






Re: Plasma Welcome Center on KDEReview

2022-09-17 Thread Albert Astals Cid
El divendres, 16 de setembre de 2022, a les 23:40:08 (CEST), Nate Graham va 
escriure:
> On 9/16/22 15:31, Albert Astals Cid wrote:
> > Those screenshots show me the Wayland icon, please fix.
> 
> Sure, how do I fix that exactly?
> 
> > I see the app has a Messages.sh but I remain unconvinced that the
> > generated
> > welcomecenter.po generated file is used.
> 
> Is there something that needs to be changed here? I'm not very familiar
> with how localization is set up, so any tips would be helpful.

Please refer to our discussion yesterday on IRC.

Understanding that this is a standalone app and none of its contents are 
reused to be shown in other apps a 
  KLocalizedString::setApplicationDomain("welcomecenter");
call in its main.cpp will do.

Sidenote, given the name of the app is plasma-welcome i'd really suggest 
changing the catalog name to plasma-welcome too instead of welcomecenter.

> 
> > When run on my computer i can't see any buttons on the top "toolbar" (they
> > are there since generally clicking where they should be gets the thing to
> > go next/ prev). What dependency version are you interested in knowing
> > that may be causing this?
> 
> Can you share a screenshot of how it looks for you?

https://i.imgur.com/P6kiwWD.png
> 
> Some shots in the dark?
> - Are there any relevant-looking errors printed to the console if you
> run it in a terminal window?

It's a kirigami app, sadly this means it has too many warnings 
https://pst.klgrth.io/paste/ydbmz

> - Do you have the qqc2-desktop-style framework installed?

Yes, version 5.98.0

> - Do you have qt5ct installed?

No, i don't

Cheers,
  Albert

> 
> 
> Nate






Re: Plasma Welcome Center on KDEReview

2022-09-16 Thread Albert Astals Cid
El divendres, 16 de setembre de 2022, a les 17:59:54 (CEST), Nate Graham va 
escriure:
> Hello folks!
> 
> I've been working with Aleix on an onboarding wizard for Plasma, based
> on work originally started by Felipe Kinoshita last year.
> 
> You can see some screenshots at
> https://invent.kde.org/websites/product-screenshots/-/commit/e300be66e62263c
> 63d8a85ba391bbcc1de691148.

Those screenshots show me the Wayland icon, please fix.

> 
> I'd like to target Plasma 5.27 for release and am submitting it for
> KDEReview. The repo is at https://invent.kde.org/plasma/welcome-app.

I see the app has a Messages.sh but I remain unconvinced that the generated 
welcomecenter.po generated file is used.

When run on my computer i can't see any buttons on the top "toolbar" (they are 
there since generally clicking where they should be gets the thing to go next/
prev). What dependency version are you interested in knowing that may be 
causing this?

Cheers,
  Albert

> 
> 
> Nate






Re: QCA2

2022-09-11 Thread Albert Astals Cid
El diumenge, 11 de setembre de 2022, a les 4:44:24 (CEST), Ron Murray va 
escriure:
> Hi Albert.
> 
>OK. I see that it works for a command-line program (I didn't know
> about qcatool, to be honest). Perhaps I didn't make it clear, but my
> project is a GUI program, using Qt5.

What does being command line or GUI have to do with anything? 

If qcatool can ask on the command line, you can just as well show a dialog 
instead of asking on the command line, right?

> Currently, QCA invokes the gpg
> executable (although I gather there are plans to switch to GPGME), and
> there are, as far as I know, only three ways to feed gpg with a
> passphrase when it needs one:
> 
> - Have gpg request it directly on the console, as you describe,
> 
> - Directly, on the command line (not a good idea), and
> 
> - Via gpg-agent.
> 
>gpg, when invoked manually, opens up a pinentry dialog, which
> collects the passphrase and feeds it to gpg-agent. QCA doesn't seem to
> contain the necessary assuan code to do that. Furthermore, it can't
> request for it on the console because it forces "--pinentry-mode
> loopback", which suppresses that. Besides, you don't want to use the
> console for anything when you're running a GUI program.
> 
>Since QCA invokes the gpg executable anyway, it makes more sense to
> just let gpg bring up a pinentry dialog.

That's not QCA design, the design is that the application brings up its own 
dialog if it needs it when it gets asked via the QCA::Event::Password request.

Cheers,
  Albert




Re: QCA2

2022-09-10 Thread Albert Astals Cid
El dissabte, 10 de setembre de 2022, a les 5:00:26 (CEST), Ron Murray va 
escriure:
>I'm working on a project using Qt5, GPG and QCA2, the latter because
> it can encrypt and decrypt PGP messages. This, of course, involves
> using the qca-gnupg plugin.
> 
>Encryption went fine (there's no need to sign anything (at the
> moment, anyway)). Decryption, however, presented a problem: How to get
> the password into gpg? I tried following the one example that I could
> find (eventhandlerdemo.cpp), but I could never get the PasswordAsker
> to, you know, actually ask for a password. 

Works fine here [1], i do

./bin/qcatool-qt5 message encrypt pgp P:df11

being df11 the short descriptor [2] of my key that has a passphrase, enter 
some 
text on the command line and press Ctrl+D and then run

./bin/qcatool-qt5 message decrypt pgp
paste the text on the command line that the encrypt process entered, press 
Ctrl+D

and feed it that and it ends up in the PassphrasePrompt class code asking my 
passphrase on the command line.

Cheers,
  Albert

[1] Well, it needs a fix in the qcatool code, but that's "irrelevant", the 
library code is fine.
https://invent.kde.org/libraries/qca/-/merge_requests/89/diffs

[2] you can use 
  qcatool-qt5 keystore list-stores
and
  qcatool-qt5 keystore list ID_OF_THE_GPG_KEYRING
to try to find your short id if needed

> I did discover, however,
> that if I first used gpg to decrypt something (and supplying my
> password to the agent in the process), that my program would
> successfully decrypt things until the agent timed out (i.e. ten minutes
> or so).
> 
>I began to think  that the problem lay in the qca2 library. I went
> through the source code and did a bit of tracing, and I found that QCA
> always supplies "--pinentry-mode loopback" on the gpg command line.
> This will never invoke the pinentry dialog, because that mode forces
> gpg to ask for a password on the command line, which, apart from being
> useless in a GUI application, won't work anyway because QCA also
> supplies "--no-tty" on the command line, and that suppresses console
> output.
> 
>I managed to modify the qca-gnupg plugin code to replace "--
> pinentry-mode loopback" with "--pinentry-mode default" when it's
> decrypting or signing a message, built the libraries, installed it, and
> now I get a proper pinentry dialog when I want to decrypt a message.
> 
>So, the questions that I have are these:
> 
> 1. I don't think that QCA, on its own, has any way to supply a password
> to gpg or gpg-agent (apart, I suppose, by supplying it on the command
> line, and nobody wants that), and anyway it's not implemented. But have
> I missed something? Has anyone got QCA to decrypt files with GPG
> lately?
> 
> 2. Would this patch be useful for others? Note that it only affects the
> qca-gnupg plugin: the rest of QCA is untouched.
> 
>I'm using the current QCA version on Debian testing (2.3.4-1+b1).






Re: Gitlab CI Dashboards and retirement of build.kde.org

2022-08-27 Thread Albert Astals Cid
El dissabte, 27 d’agost de 2022, a les 11:44:47 (CEST), Ben Cooksley va 
escriure:
> Hi all,
> 
> This evening I completed the necessary setup required to complete our
> Gitlab CI dashboards, which can now be found at
> https://metrics.kde.org/dashboards/f/aNxvXJW4k/gitlab-ci (KDE Developer
> account login required)
> 
> These allow any developer to get a view on the current CI status of
> projects and groups of projects on a branch and platform basis - and should
> hopefully provide useful insight into the overall status that can currently
> be obtained from Jenkins.
> 
> As this was the last thing holding us back from shutting down build.kde.org,
> i'd like to proceed with retiring and shutting down build.kde.org as soon
> as possible so the capacity it occupies can be released and reallocated to
> Gitlab.
> 
> If anyone would like to experiment with customised views for their own
> purposes (where the above provided ones are insufficient) please file a
> Sysadmin ticket.
> 
> Please let me know if there are any questions on the above.

Looks great.

One thing that i'm not sure i understand correctly, currently in the General 
Overview, it says that there are 3 projects currently failing kwin, kpackage 
and kphotoalbum, but then if i go to the Per Platform View i get that rkward 
is failing on windows. Shouldn't rkward also be listed as failing on the 
general overview?

Cheers,
  Albert

> 
> Thanks,
> Ben






Re: Aura Browser in KDE Review

2022-08-26 Thread Albert Astals Cid
El divendres, 26 d’agost de 2022, a les 12:27:22 (CEST), Aditya Mehra va 
escriure:
> Hi,
> 
> Aura Browser is in KDE Review and would like to release it along with Plasma
> Bigscreen
> 
> Aura browser is a web browser specifically designed to work with remote
> controls and key navigation for Plasma Bigscreen, it supports a virtual
> mouse cursor and multiple tabs it also includes a third party AdBlock from
> brave browser based on easy list.
> 
> You can find the repository here:
> https://invent.kde.org/plasma-bigscreen/aura-browser
> 
> Request to please review Aura Browser.

wcgrep text: | grep \"
will show you lots of places that need i18n (or qstr that you use in 2 places)

You have a Messages.sh but if you're going to keep using qstr you need to 
update it to use the Qt extraction command not the ki18n one.

All those third-party are a bit sub-optimal, can we depend on packaged 
versions of them or they don't exist as such?

Can you make cmake complain if the virtualkeyboard is not found? i think we 
have RUNTIME warning categories for that?

It's relatively easy to get it crashing on closing the app, open a webpage (i 
tried wikipedia), close the app

gdb trace https://pst.klgrth.io/paste/jobc7

Cheers,
  Albert

> 
> Regards,
> Aditya






Re: Plasma Remote Controllers in KDE Review

2022-08-26 Thread Albert Astals Cid
El divendres, 26 d’agost de 2022, a les 12:21:40 (CEST), Aditya Mehra va 
escriure:
> Hi,
> 
> Plasma remote controllers is in KDE Review and would like to release it
> along with Plasma Bigscreen
> 
> Plasma remote controllers allows translating various input device events
> into keyboard and pointer events send from remote control type devices like
> CEC and Gamepads to provide key navigation support in applications. It also
> allows re mapping of keys to specific events.
> 
> You can find the repository here:
> https://invent.kde.org/plasma-bigscreen/plasma-remotecontrollers
> 
> Request to please review Plasma remote controllers.

There's a few "can be marked override" that i think that you should.

You need to call KLocalizedString::setApplicationDomain in the main of the 
app.

Cheers,
  Albert

> 
> Regards,
> Aditya






Re: Plank Player in KDE Review

2022-08-26 Thread Albert Astals Cid
El divendres, 26 d’agost de 2022, a les 12:29:31 (CEST), Aditya Mehra va 
escriure:
> Hi,
> 
> Plank Player is in KDE Review and would like to release it along with Plasma
> Bigscreen
> 
> Plank Player is a local media player specifically designed to work with
> remote controls and key navigation for bigscreen.
> 
> You can find the repository here:
> https://invent.kde.org/plasma-bigscreen/plank-player
> 
> Request to please review Plank Player.

This seems wrong
MimeType=application/vnd.debian.binary-package;application/x-rpm;

This needs i18n / or qstr
   ./app/qml/Menu.qml:91:text: "Press 'esc' or the [←] Back 
button to close"
and then you need the appropriate a Messages.sh


I'm not sure having the folders sorted case sensitive makes sense, i think we 
stopped doing that a while ago?

Cheers,
  Albert

> 
> Regards,
> Aditya






Re: Request to bump QT_MIN_VERSION to 5.15.2 for whole Plasma

2022-06-27 Thread Albert Astals Cid
For some reason Ömer decided to send the email to two different places but 
without CC, this has been handled in kde-devel ml

Cheers,
  Albert

El diumenge, 26 de juny de 2022, a les 14:06:09 (CEST), Ömer Fadıl USTA va 
escriure:
> Hello everyone,
> 
> Right now plasma projects' minimum depends on KF >5.90 and after KF5.86 its
> REQUIRED_QT_VERSION is 5.15.2
> thus keeping QT_MIN_VERSION as 5.15.0 is meaningless and it also causes
> problems such as in this merge :
> https://invent.kde.org/utilities/konsole/-/merge_requests/689
> 
> So I am suggesting to bump all plasma project's QT_MIN_VERSION to 5.15.2
> I will be glad to get some replies and want to hear what are your opinions
> about this
> 
> Thank you
> 
> Ömer Fadıl Usta
> PGP key : 0xfd11561976b1690b
> about.me/omerusta






Re: Eloquens now on KDEREVIEW

2022-06-20 Thread Albert Astals Cid
El dilluns, 20 de juny de 2022, a les 22:30:06 (CEST), Felipe Kinoshita va 
escriure:
> For those who don't know, Eloquens is a simple application targeted at
> developers/designers, it generates the lorem ipsum text and allows you to
> customize it a little bit like adding heading, bullet lists, etc...

You need to load the translation catalog, aka 
KLocalizedString::setApplicationDomain

You're using invent for bug reporting which is "not allowed"


You don't need 
  QCoreApplication::setApplicationName(QStringLiteral("eloquens"));
KAboutData::setApplicationData will do it for you

> 
> I would also like to ask if it would be possible for it to be released
> along with KDE Gear.

Personally I always recommend new apps to release a few times on their own 
because it allows them to do a much more direct feedback look with users, 
otherwise your first release is in 2 months and if users think "this is cool 
but would be much cooler with X feature" (which is relatively common on new 
apps), they have to wait 4 months more.

On the other hand this application is super simple, so I may be convinced 
that's not gonna happen.

Cheers,
  Albert

> 
> Thanks,
> Felipe






Re: KPipeWire in kdereview

2022-05-30 Thread Albert Astals Cid
El dilluns, 30 de maig de 2022, a les 14:55:55 (CEST), Aleix Pol va escriure:
> Hi,
> I'd like to get KPipeWire (https://invent.kde.org/plasma/kpipewire)
> released from KDE eventually.
> 
> At the moment it's under Plasma as it's the only place where it's
> being used, I know we might want to use it in spectacle eventually,
> although I feel like it's premature to get it in frameworks just yet,
> although it should be a possibility down the line.
> 
> If you wanted to test it beyond what Plasma does, you can try this
> little app for recording your wayland desktops and windows.
> https://invent.kde.org/apol/screenrecord

It needs a Messages.sh

This signal is never emited?
  void cursorModeChanged(Screencasting::CursorMode cursorMode);

This property NOTIFY signal is wrong?
  Q_PROPERTY(QString outputName READ outputName WRITE setOutputName NOTIFY 
uuidChanged)


The 3 signals in ScreencastingStream are never emitted?

PipeWireCore header is installed but not exported. If you export it, it will 
need a d-pointer

PipeWireSourceItem is exported but has no d-pointer

> 
> Cheers!
> Aleix
> 






Re: kdereview Telly Skout

2022-05-12 Thread Albert Astals Cid
El dimarts, 10 de maig de 2022, a les 20:02:07 (CEST), Plata va escriure:
> Hi,
> 
> 
> thanks for the feedback.

Please keep kde-core-devel in CC

> 
> Yes, I've generally planned to support multiple sources (not only TV 
> Spielfilm). Do you have a particular one you'd like to have?

Not particularly, no.

> I've tried to add KCrash 
> (https://invent.kde.org/plasma-mobile/telly-skout/-/merge_requests/7) 
> but it fails the CI. Currently, I don't know why.

You need to add kcrash to 
https://invent.kde.org/plasma-mobile/telly-skout/-/blob/master/.kde-ci.yml

> 
> I'm investigating the crash. I've opened 
> https://invent.kde.org/plasma-mobile/telly-skout/-/issues/12 to track it 
> and I can reproduce it but couldn't find the error yet.

Sent https://invent.kde.org/plasma-mobile/telly-skout/-/merge_requests/8 your 
way.

Cheers,
  Albert

> 
> 
> Am 09.05.22 um 23:29 schrieb Albert Astals Cid:
> > El dilluns, 9 de maig de 2022, a les 18:38:05 (CEST), Plata va escriure:
> >> Hi,
> >>
> >>
> >> after my TV guide application Telly Skout has been moved to
> >> https://invent.kde.org/plasma-mobile/telly-skout, I'm asking for
> >> kdereview (according to
> >> https://community.kde.org/Policies/Application_Lifecycle#kdereview).
> > Are there plans to support anything els than Germany?
> >
> > Please add
> >KCrash::initialize
> > The thing just crashed on me and didn't give me the expected "i have 
> > crashed" dialog.
> >
> > Valgrind trace of the crash https://ghostbin.com/WP3X8
> >
> > Cheers,
> >Albert
> >
> >> Please open issues in GitLab if you find problems/possibilities for
> >> improvement.
> >>
> >>
> >> Thanks
> >>
> >> Plata
> >>
> >>
> >
> >
> >
> 






Re: kdereview Telly Skout

2022-05-09 Thread Albert Astals Cid
El dilluns, 9 de maig de 2022, a les 18:38:05 (CEST), Plata va escriure:
> Hi,
> 
> 
> after my TV guide application Telly Skout has been moved to 
> https://invent.kde.org/plasma-mobile/telly-skout, I'm asking for 
> kdereview (according to 
> https://community.kde.org/Policies/Application_Lifecycle#kdereview).

Are there plans to support anything els than Germany?

Please add 
  KCrash::initialize
The thing just crashed on me and didn't give me the expected "i have crashed" 
dialog.

Valgrind trace of the crash https://ghostbin.com/WP3X8

Cheers,
  Albert

> 
> Please open issues in GitLab if you find problems/possibilities for 
> improvement.
> 
> 
> Thanks
> 
> Plata
> 
> 






Re: RKWard is in kdereview - again

2022-03-27 Thread Albert Astals Cid
El dilluns, 28 de març de 2022, a les 0:09:38 (CEST), Albert Astals Cid va 
escriure:
> El dissabte, 26 de març de 2022, a les 21:34:06 (CEST), Thomas 
> Friedrichsmeier va escriure:
> > Hi!
> > 
> > KDE.org has been our home for a 7.5(!) years, now (after over a
> > decade on sourceforge), but we still haven't left playground... After a
> > lot of procrastination on that matter, a previous review failed due to
> > lack of time on my part. Sorry! Now, finally, I'd like to ask you to
> > start reviewing RKWard for inclusion into exragear / education, once
> > more.
> > 
> > RKWard is an easy to use and easily extensible IDE/GUI for R (a
> > powerful language and environment for statistical computing, if you
> > did not know it, yet). It aims to combine the power of the
> > R-language with the ease of use of commercial statistics tools.
> > 
> > RKWard is used productively on Linux/BSD, Mac, and Windows.
> > 
> > 
> > 
> > Here's a summary of the critical comments we got in the previous round
> > of review (https://marc.info/?l=kde-core-devel&m=153883367600690&w=2):
> > 
> > Albert Astals Cid remarked
> > (https://marc.info/?l=kde-core-devel&m=153912336114603&w=2):
> > 
> > > the i18n folder seems like from the past and something you should not
> > > need if in kde infrastructure. Could you delete it?
> > 
> > We cannot easily get rid of this, entirely, as we are shipping
> > (non-compiled) plugins that keep their message catalogs in relative
> > paths, and thus we have to install .mo files to custom locations. Pino
> > Toscano has helped to bring this more in line with standard KDE.org
> > processes (https://invent.kde.org/education/rkward/-/merge_requests/2).
> > 
> > > Also I'd suggest you compile enabling
> > > -DECM_ENABLE_SANITIZERS="address;undefined"
> > 
> > Thanks for the hint, did not know that switch. One effect of this is a
> > lot of false positive "runtime errors", when casting a half-destroyed
> > QObject* to its original (derived) type, in order to remove it from
> > containers (e.g. a QHash of KActionCollection()s). Is there an elegant
> > way around this?
> > 
> > > There's a few memory leaks (reported at exit) that you may want to
> > > have a look.
> > 
> > I fixed a lot of that, but didn't work out all of them.
> > 
> > > And there's also a few undefined behaviour warnings on exit, you've
> > > them marked as "known" things but it'd be good if you could find a way
> > > to fix them.
> > 
> > Not quite sure what you were referring to, esp. what I may have marked
> > as known behavior. I did fix several quirks at exit since this comment
> > (and I keep introducing new ones, all the time).
> > 
> > > Your help menu is for some reason missing the Change Language option,
> > > i tried to do it a quick fix but could not, i would appreciate if you
> > > could find a way to only define the extra actions and not all of them
> > > (like we do for example in okular).
> > 
> > I've added the switch application language option (in Settings rather
> > than Help). Our help menu is perhaps more customized than meets the eye
> > on first glance. For instance, we have an app-integrated help system
> > instead of external handbook (at least as the primary documentation),
> > and our bug report dialog is beefed up to include a lot of extra
> > information by default (mostly importantly: version of R). I guess it
> > would be possible to hack KHelpMenu this way, but at least at some
> > point in the past it seemed cleaner to implement "from scratch" (but
> > using the default actions, where applicable).
> > 
> > 
> > 
> > Comments by Jonathan Riddel
> > (https://marc.info/?l=kde-core-devel&m=153916691226377&w=2):
> > 
> > > It installs two desktop files which creates duplicate menu entries
> > > /usr/share/applications/org.kde.rkward-open.desktop
> > > /usr/share/applications/org.kde.rkward.desktop
> > 
> > Completed following suggestions by Thomas Baumgart and Meik Michalke:
> > https://marc.info/?l=kde-core-devel&m=153942961310071&w=2
> > 
> > > The .desktop files call it a "GUI for R" which is not a great
> > > description, everything in the menu is a GUI.  I recommend "R
> > > Statistical Programming" or "IDE for R" maybe.
> > 
> > Renamed to Statistics with R, following Meik&#

Re: RKWard is in kdereview - again

2022-03-27 Thread Albert Astals Cid
El dissabte, 26 de març de 2022, a les 21:34:06 (CEST), Thomas Friedrichsmeier 
va escriure:
> Hi!
> 
> KDE.org has been our home for a 7.5(!) years, now (after over a
> decade on sourceforge), but we still haven't left playground... After a
> lot of procrastination on that matter, a previous review failed due to
> lack of time on my part. Sorry! Now, finally, I'd like to ask you to
> start reviewing RKWard for inclusion into exragear / education, once
> more.
> 
> RKWard is an easy to use and easily extensible IDE/GUI for R (a
> powerful language and environment for statistical computing, if you
> did not know it, yet). It aims to combine the power of the
> R-language with the ease of use of commercial statistics tools.
> 
> RKWard is used productively on Linux/BSD, Mac, and Windows.
> 
> 
> 
> Here's a summary of the critical comments we got in the previous round
> of review (https://marc.info/?l=kde-core-devel&m=153883367600690&w=2):
> 
> Albert Astals Cid remarked
> (https://marc.info/?l=kde-core-devel&m=153912336114603&w=2):
> 
> > the i18n folder seems like from the past and something you should not
> > need if in kde infrastructure. Could you delete it?
> 
> We cannot easily get rid of this, entirely, as we are shipping
> (non-compiled) plugins that keep their message catalogs in relative
> paths, and thus we have to install .mo files to custom locations. Pino
> Toscano has helped to bring this more in line with standard KDE.org
> processes (https://invent.kde.org/education/rkward/-/merge_requests/2).
> 
> > Also I'd suggest you compile enabling
> > -DECM_ENABLE_SANITIZERS="address;undefined"
> 
> Thanks for the hint, did not know that switch. One effect of this is a
> lot of false positive "runtime errors", when casting a half-destroyed
> QObject* to its original (derived) type, in order to remove it from
> containers (e.g. a QHash of KActionCollection()s). Is there an elegant
> way around this?
> 
> > There's a few memory leaks (reported at exit) that you may want to
> > have a look.
> 
> I fixed a lot of that, but didn't work out all of them.
> 
> > And there's also a few undefined behaviour warnings on exit, you've
> > them marked as "known" things but it'd be good if you could find a way
> > to fix them.
> 
> Not quite sure what you were referring to, esp. what I may have marked
> as known behavior. I did fix several quirks at exit since this comment
> (and I keep introducing new ones, all the time).
> 
> > Your help menu is for some reason missing the Change Language option,
> > i tried to do it a quick fix but could not, i would appreciate if you
> > could find a way to only define the extra actions and not all of them
> > (like we do for example in okular).
> 
> I've added the switch application language option (in Settings rather
> than Help). Our help menu is perhaps more customized than meets the eye
> on first glance. For instance, we have an app-integrated help system
> instead of external handbook (at least as the primary documentation),
> and our bug report dialog is beefed up to include a lot of extra
> information by default (mostly importantly: version of R). I guess it
> would be possible to hack KHelpMenu this way, but at least at some
> point in the past it seemed cleaner to implement "from scratch" (but
> using the default actions, where applicable).
> 
> 
> 
> Comments by Jonathan Riddel
> (https://marc.info/?l=kde-core-devel&m=153916691226377&w=2):
> 
> > It installs two desktop files which creates duplicate menu entries
> > /usr/share/applications/org.kde.rkward-open.desktop
> > /usr/share/applications/org.kde.rkward.desktop
> 
> Completed following suggestions by Thomas Baumgart and Meik Michalke:
> https://marc.info/?l=kde-core-devel&m=153942961310071&w=2
> 
> > The .desktop files call it a "GUI for R" which is not a great
> > description, everything in the menu is a GUI.  I recommend "R
> > Statistical Programming" or "IDE for R" maybe.
> 
> Renamed to Statistics with R, following Meik's suggestion
> (https://marc.info/?l=kde-core-devel&m=153942595709279&w=2).
> 
> > I tidied up the files with the icon licence as they could easily be
> > lost.
> 
> Done by Jonathan.
> 
> > It depends on WebKit which is not supported, could this be ported to
> > WebEngine?
> 
> Meanwhile, RKWard can be compiled to use QtWebEngine (and this is the
> default), but support for webkit has not been dropped, because MinGW is
> an important platform for us.
> 
> >

Re: KSANECore in KDE review

2022-03-27 Thread Albert Astals Cid
El diumenge, 27 de març de 2022, a les 18:29:18 (CEST), Alexander Stippich va 
escriure:
> Hello everyone,
> 
> KSANECore is now in KDE review. Kåre and I mentioned it in previous emails 
> before, but as a short summary:
> KSANECore is a Qt interface to the SANE scanner library. It is stripped out 
> of 
> the KSaneWidget of libksane without any QWidget dependency. It is currently  
> located inside the libksane repository as KSaneCore and basically just copied 
> into the new repository.
> 
> Due to breaking API anyway, the code was cleaned up, better named as well as 
> smaller API fixes made on top. Also, KSANECore is fully REUSE compliant.
> KSaneWidget of libksane will remain ABI compatible.
> 
> I don't know if it is strictly required to move the new repo with already 
> existing code through KDE review, but I guessed it is better to be on the 
> safe 
> side :) 


Ran it through clang-tidy + my preferred options, came with a few suggestions, 
nothing earth shattering but i think it makes sense, if you don't, that's fine, 
attached the file with the results, feel free to ask if you don't understand 
what some warnings say


The only other question i have is if you think that it may make sense to hide 
the members of DeviceInfo behind a d-pointer in case it ever needs more and 
that would be an ABI change (not sure which kind of ABI promises you want to 
have for this library).


Ok, I lied, I have another question, what's the goal for the library? 
independently released? KDE Gear? KDE Frameworks?

Cheers,
  Albert

> 
> The plan is to switch libksane and Skanpage over to the new library during 
> the 
> KDE Gear 22.08 release cycle. The adaptions are located at
> https://invent.kde.org/astippich/skanpage/-/commits/ksanecore
> and
> https://invent.kde.org/astippich/libksane/-/commits/ksanecoreSplit
> 
> Best regards,
> Alex
> 
> 
> 
> 

ksanecore/src/coreinterface.h:113:5: error: single-argument constructors must 
be marked explicit to avoid unintentional implicit conversions 
[google-explicit-constructor,-warnings-as-errors]
CoreInterface(QObject *parent = nullptr);
^
explicit

ksanecore/src/scanthread.h:43:5: error: single-argument constructors must be 
marked explicit to avoid unintentional implicit conversions 
[google-explicit-constructor,-warnings-as-errors]
ScanThread(SANE_Handle handle);
^
explicit

ksanecore/src/coreinterface_p.h:35:5: error: single-argument constructors must 
be marked explicit to avoid unintentional implicit conversions 
[google-explicit-constructor,-warnings-as-errors]
CoreInterfacePrivate(CoreInterface *parent);
^
explicit

ksanecore/src/coreoption.h:73:5: error: single-argument constructors must be 
marked explicit to avoid unintentional implicit conversions 
[google-explicit-constructor,-warnings-as-errors]
CoreOption(QObject *parent = nullptr);
^
explicit

ksanecore/src/internaloption.h:22:5: error: constructors that are callable with 
a single argument must be marked explicit to avoid unintentional implicit 
conversions [google-explicit-constructor,-warnings-as-errors]
InternalOption(BaseOption *option, QObject *parent = nullptr);
^
explicit




ksanecore/src/coreinterface.cpp:35:26: error: use std::make_unique instead 
[modernize-make-unique,-warnings-as-errors]
: QObject(parent), d(std::unique_ptr(new 
CoreInterfacePrivate(this)))
 ^~~   
~~
 std::make_unique

ksanecore/src/coreoption.cpp:16:62: error: use std::make_unique instead 
[modernize-make-unique,-warnings-as-errors]
CoreOption::CoreOption(QObject *parent) : QObject(parent), 
d(std::unique_ptr(new OptionPrivate()))
 ^~~
~~~
 std::make_unique




ksanecore/src/coreinterface.cpp:133:98: error: the parameter 'userName' is 
copied for each invocation but only used as a const reference; consider making 
it a const reference [performance-unnecessary-value-param,-warnings-as-errors]
CoreInterface::OpenStatus CoreInterface::openRestrictedDevice(const QString 
&deviceName, QString userName, QString password)

 ^

 const  &

ksanecore/src/coreinterface.cpp:133:116: error: the parameter 'password' is 
copied for each invocation but only used as a const reference; consider making 
it a const reference [performance-unnecessary-value-param,-warnings-as-errors]
CoreInterface::OpenStatus CoreInterface::openRestrictedDevice(const QString 
&deviceName, QString userName, QString password)

   ^
  

Re: KQuickChatComponents in kdereview

2022-01-17 Thread Albert Astals Cid
El dilluns, 17 de gener de 2022, a les 21:22:21 (CET), Janet Blackquill va 
escriure:
> Am Sa., 1. Jan. 2022 um 19:08 Uhr schrieb Albert Astals Cid :
> >
> > El divendres, 31 de desembre de 2021, a les 5:21:44 (CET), Janet Blackquill 
> > va escriure:
> > > Hi,
> > >
> > > KQuickChatComponents
> > > (https://invent.kde.org/libraries/kquickchatcomponents) is in
> > > kdereview now.
> > >
> > > It is a very small (as of now) library mostly containing a base bubble
> > > component offering a consistent bubble design, with a few variants
> > > built on top of the bubble offering common patterns of chat things
> > > with bubble backgrounds and layouts. Besides the timestamp component,
> > > that's pretty much it, since chat apps need to diverge heavily to
> > > accomodate their protocols. This library's purpose is mostly for
> > > getting KDE's plethora of QtQuick chat apps to converge on one (1)
> > > bubble design instead of a different look per app, as well as other
> > > details like the selection of shortcuts used for common chat app
> > > actions (see MR !2)
> >
> > It needs a Messages.sh + ecm_create_qm_loader for that qsTr
> 
> What exactly does the ecm_create_qm_loader do?

You can see the documentation here 
https://api.kde.org/ecm/module/ECMPoQmTools.html

Anything you don't understand?

> Also note that KQCC
> supports more than one build system, so we can't rely on build-system
> specific magic for proper functionality.

My suggestion? Don't do that, you're just shooting yourself on the foot.

Having multiple outbound supported buildsystems is nice, but having multiple 
build systems to build yourself only is headache for no real reason.

Cheers,
  Albert

> 
> > Running the TextBubble.qml example warns me about implicitHeight binding 
> > loops
> 
> Fixed.
> 
> > Cheers,
> >   Albert
> >
> > >
> > > -- Janet
> > >
> >
> >
> >
> >
> 






Re: KQuickChatComponents in kdereview

2022-01-01 Thread Albert Astals Cid
El divendres, 31 de desembre de 2021, a les 5:21:44 (CET), Janet Blackquill va 
escriure:
> Hi,
> 
> KQuickChatComponents
> (https://invent.kde.org/libraries/kquickchatcomponents) is in
> kdereview now.
> 
> It is a very small (as of now) library mostly containing a base bubble
> component offering a consistent bubble design, with a few variants
> built on top of the bubble offering common patterns of chat things
> with bubble backgrounds and layouts. Besides the timestamp component,
> that's pretty much it, since chat apps need to diverge heavily to
> accomodate their protocols. This library's purpose is mostly for
> getting KDE's plethora of QtQuick chat apps to converge on one (1)
> bubble design instead of a different look per app, as well as other
> details like the selection of shortcuts used for common chat app
> actions (see MR !2)

It needs a Messages.sh + ecm_create_qm_loader for that qsTr 

Running the TextBubble.qml example warns me about implicitHeight binding loops

Cheers,
  Albert

> 
> -- Janet
> 






Re: Moving kio-upnp-ms to unmaintained?

2022-01-01 Thread Albert Astals Cid
El dissabte, 1 de gener de 2022, a les 22:44:54 (CET), David Faure va escriure:
> The kio-upnp-ms repo, described by the repo-metadata entry below,
> hasn't been ported to Qt5/KF5, AFAICS. By now I think this means it's 
> completely dead. Can we move it to unmaintained, to avoid confusion
> when grepping around for code to port away from deprecated KF5 methods?

Yes.

Cheers,
  Albert

> 
> description: KIO Slave for UPnP MediaServer devices
> hasrepo: true
> identifier: kio-upnp-ms
> name: KIO UPnP MediaServer
> projectpath: playground/base/kio-upnp-ms
> repoactive: true
> repopath: network/kio-upnp-ms
> 
> 






Re: Announcing General Availability of Gitlab CI for Linux, FreeBSD and Android

2021-11-28 Thread Albert Astals Cid
El dissabte, 27 de novembre de 2021, a les 19:30:08 (CET), Ben Cooksley va 
escriure:
> Hi all,
> 
> As mentioned in the subject, i'm happy to announce the general availability
> of native Gitlab CI for all KDE projects for Linux, FreeBSD and Android.
> 
> For those who would like to get their projects setup, please ensure you
> first have a .kde-ci.yml file in the root of your repository specifiying
> the necessary dependencies of your project and any options your project
> requires to build and conduct tests. You can find a list of all the
> supported options at
> https://invent.kde.org/sysadmin/ci-utilities/-/blob/master/config-template.yml
> along with guidelines on how to specify your dependencies.
> 
> Please note the following when specifying the branch to use:
> - @same should be used for projects which are released together and which
> follow the same branching scheme (release/21.08 for instance)
> - @stable should be used to refer to the current stable branch
> - @latest should be used to refer to the current development branch
> 
> While the system has been seeded based on the current dependency metadata,
> if this was incorrect for your project then you may find that the system
> gives an error that it cannot locate the dependency when first run. If this
> happens to you then please look into submitting a merge request to
> https://invent.kde.org/sysadmin/ci-management/-/tree/master/seeds so that
> this dependency can be included in future seed runs.
> 
> Once you have your .kde-ci.yml file setup you can then setup your
> .gitlab-ci.yml to include the appropriate platform files from
> https://invent.kde.org/sysadmin/ci-utilities/-/tree/master/gitlab-templates.
> 
> Please note that currently this is limited to builds for master and other
> development branches, as the necessary metadata for @stable is not yet
> fully in place. This information can be found at
> https://invent.kde.org/sysadmin/repo-metadata/-/blob/master/branch-rules.yml
> - it would be appreciated if developers could please look into updating
> this for their projects at the same time.
> 
> With regards to Windows builds, they should become available soon - there
> is still some infrastructural work that needs to be completed on our Docker
> image, as well as the facilities to build those images which needs to be
> completed prior to these being available. We will be transitioning to using
> Docker for our Windows builds which will hopefully eliminate many of the
> issues we've had with lingering processes that have plagued the Jenkins
> system.
> 
> With the availability of these Gitlab builds we are now entering the
> twilight phase of our Jenkins based system at build.kde.org so it is
> imperative that developers setup their CI using the new tooling. For those
> using the legacy Jenkins system with Gitlab (using the templates at
> https://invent.kde.org/sysadmin/ci-tooling/-/tree/master/invent) these will
> be discontinued at the same time as build.kde.org, so you will need to
> migrate to the new tooling as well.
> 
> Please let us know if you have any questions on the above.

Do we have any plan for "category pages" [1] like in Jenkins?

Cheers,
  Albert

[1] i.e. something like 
https://build.kde.org/job/Applications/view/Platform%20-%20WindowsMSVCQt5.15/

> 
> Thanks,
> Ben
> 






Re: Submitting SubtitleComposer for KDE Review

2021-11-07 Thread Albert Astals Cid
El diumenge, 7 de novembre de 2021, a les 19:05:44 (CET), Mladen Milinkovic va 
escriure:
> Have updated the bug report address to bugs.kde.org (default) in KAboutData 
> and project has been added.
> 
> Should I do something else to complete the process?

From my side I think you fixed everything I reported.

I'd say wait a week more in case someone else has time to have a look, but if 
there's no more movement consider the review done and ask sysadmin to finish 
the move to its final location.

Cheers,
  Albert

> 
> Cheers,
>   Mladen
> 
> On 10/16/21 00:28, Mladen Milinkovic wrote:
> > On 10/15/21 22:39, Albert Astals Cid wrote:
> >> I think you may have dropped k-c-d from the CC, adding it back.
> >
> > I have hit a wrong button again :)
> >
> >
> >> El divendres, 15 d’octubre de 2021, a les 22:18:00 (CEST), Mladen 
> >> Milinkovic va escriure:
> >>> On 10/13/21 23:03, Albert Astals Cid wrote:
> >>>> I think the tests are somehow not correctly flagged as tests, running 
> >>>> ctest will only run the appstream check and 
> >>>> src/tests/test-subtitle, but not test-core-rangelist and the rest.
> >>>
> >>> Fixed in ec9ffba - I'm pretty sure this was working fine at some point in 
> >>> (not so distant) past.
> >>>
> >>>
> >>>> The first text format change doesn't seem to trigger the "file has 
> >>>> changed and we should enable saving" logic. i.e. 
> >>>> i have written a new subtitle line that says "HOLA" and saved the 
> >>>> subtitle. Now if i select all the text and press 
> >>>> the strikeout button, the save button does not get enabled, if i press 
> >>>> the strikeout button again, the save button 
> >>>> correctly gets enabled.
> >>>
> >>> I believe this was happening sometimes due to "relatively scary valgrind 
> >>> warning" below... looks like it's not 
> >>> happening
> >>> anymore - could you please confirm?
> >>
> >> Seems to be working now :)
> >>
> >>>
> >>>
> >>>> If i close a video while it's playing, the Play button will still be 
> >>>> enabled (if i stop the video it will not)
> >>>
> >>> Fixed in 663d209
> >>>
> >>>
> >>>> Opening a .srt i just created and editing one of the subtitle lines i 
> >>>> get this relatively scary valgrind 
> >>>> warninghttps://ghostbin.com/YGnmL
> >>>
> >>> Fixed in 663d209. QUndoStack::push(action) can merge and delete action, 
> >>> in those cases it ended with invalid read
> >>> immediately afterwards.
> >>>
> >>>
> >>>> When opening an existing .srt, there is a few Subtitle::insertLine calls 
> >>>> that end up calling 
> >>>> Subtitle::processAction with the if(app()->subtitle() != this) 
> >>>> situation. I think all those Actions leak, because 
> >>>> you just call redo on them but they are not deleted by anyone, no?
> >>>
> >>> Yes they were leaking fixed them with 911b94b.
> >>>
> >>> There are still some definite leaks after closing application:
> >>>- libfontconfig/QTextDocument (FcFontRenderPrepare 
> >>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989)
> >>>- QXcbGlxWindow::createVisual() calling radeon_dri.so and 
> >>> amdgpu_winsys_create()
> >>>- Breeze::WidgetStateEngine::registerWidget calling QObject::connect 
> >>> (might be related to
> >>> "QCoreApplication::postEvent: Unexpected null receiver" messages at 
> >>> application shutdown)
> >>>- KF5WidgetAddons (KSelectActionPrivate::init())
> >>>
> >>> There are also some memory errors that seem caused by KIO/KUrlRequester.
> >>>
> >>> There are some possible leaks related to QTextDocument and rendering, 
> >>> will investigate ASAP if it's due to something
> >>> that SC does wrong.
> >>
> >> I think most of those are not your fault, but if you can spend a bit of 
> >> time investigating won't hurt :)
> >
> > Sure... will do that at some point soonish.
> >
> >
> >>>> The "Report bug" incorrectly links 
> >>>> tohttps://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of 
> >>>> bug.kde.org
> >>>
> >>> I didn't change the bug report url to bugs.kde.org yet as it doesn't seem 
> >>> possible to file Subtitle Composer bugs 
> >>> there?
> >>> Would prefer to change it right before SC gets included there if it's 
> >>> necessary. There are SC binaries that get
> >>> generated pretty often that people are using - I'd like them to have a 
> >>> bug report url they can use to report bugs.
> >>
> >> Ok, then should open a request at 
> >> https://phabricator.kde.org/maniphest/task/edit/form/2/ so that a 
> >> subtitlecomposer 
> >> product is created :)
> >
> > And done https://phabricator.kde.org/T14947
> >
> >
> >> Cheers,
> >>Albert
> >>
> >>>
> >>>>
> >>>> Cheers,
> >>>> Albert
> >>>
> >>> Thank you!
> >>>
> >>
> >
> > Cheers
> 
> 
> 






Re: Submitting Kalendar for KDE Review

2021-10-26 Thread Albert Astals Cid
El dimarts, 26 d’octubre de 2021, a les 1:09:40 (CEST), Claudio Cambra va 
escriure:
> Hi Albert,
> 
> I forgot to hit the reply all button... thanks for ccing k-c-d back.
> 
> Now that the two-week review period is over, I wanted to ask if it is
> possible for Kalendar to be included in KDE Gear 21.12?

Please email release-t...@kde.org about this :)

Cheers,
  Albert

> 
> I'd also like to ask what your thoughts would be on having a "beta" or
> "pre-release" release at the end of next week? This could really help us
> gather bug reports and patch up as much stuff as possible before December.
> 
> Thanks for your help!
> 
> Clau
> 
> On Mon, Oct 18, 2021 at 10:51 AM Albert Astals Cid  wrote:
> 
> > I think you may have dropped k-c-d from the CC, adding it back.
> >
> >
> > El dissabte, 16 d’octubre de 2021, a les 14:18:08 (CEST), Claudio Cambra
> > va escriure:
> > > Hi Albert,
> > >
> > > Thanks for your thorough review, and sorry about the late reply. We've
> > been
> > > working on addressing these issues and have fixed them:
> > >
> > > - Q_PROPERTY compilation warnings are now fixed
> > > - persistentIndex in the todomodel class are now QModelIndex
> > > - We now provide vendored copies of the components we require from
> > Kirigami
> > > Addons (for now, until KA passes KDE Review)
> > > - We added KLocalizedString::setApplicationDomain("kalendar");
> > > - We now use standaloneMonthName for our month-year strings per your
> > > recommendation
> > > - We fixed the issue with valgrind you brought up
> > >
> > > Additionally, we went ahead and addressed the user-facing issues you
> > > commented on (thanks!):
> > > - You should now get a pointing-hand cursor when hovering over links in
> > the
> > > incidence information drawer
> > > - Clicking on empty space in the various views should now deselect the
> > > current incidence and close the incidence information drawer
> > >
> > > Once again, thank you for taking the time to review Kalendar -- if you
> > find
> > > anything else we should fix, we will be very happy to do so!
> >
> > Good work!
> >
> > Cheers,
> >   Albert
> >
> > >
> > > Clau
> > >
> > > On Mon, Oct 11, 2021 at 12:01 AM Albert Astals Cid 
> > wrote:
> > >
> > > > El diumenge, 10 d’octubre de 2021, a les 22:45:06 (CEST), Claudio
> > Cambra
> > > > va escriure:
> > > > > Hey all,
> > > > >
> > > > > We would like to move Kalendar to KDE review. We would also like to
> > ask
> > > > if
> > > > > it would be possible to be release along with KDE Gear since the KDE
> > > > > PIM libraries don't have a stable ABI, and we need to make sure we
> > > > > remain compatible with them.
> > > > >
> > > > > For those who don't know, Kalendar is a new calendar application
> > built
> > > > > with Kirigami and Akonadi. Our repo is located at
> > > > > https://invent.kde.org/pim/kalendar/. We welcome any feedback!
> > > >
> > > > These seem like important enough to fix
> > > >
> > > > AutoMoc:
> > /home/tsdgeos/devel/kde/kalendar/src/hourlyincidencemodel.h:30:
> > > > Warning: Property declaration model has no READ accessor function or
> > > > associated MEMBER variable. The property will be invalid.
> > > > AutoMoc:
> > > > /home/tsdgeos/devel/kde/kalendar/src/incidenceoccurrencemodel.h:42:
> > > > Warning: Property declaration filter has no READ accessor function or
> > > > associated MEMBER variable. The property will be invalid.
> > > > AutoMoc:
> > /home/tsdgeos/devel/kde/kalendar/src/multidayincidencemodel.h:35:
> > > > Warning: Property declaration model has no READ accessor function or
> > > > associated MEMBER variable. The property will be invalid.
> > > > AutoMoc:
> > > > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:17:
> > > > Warning: Property declaration incidenceChanger has no READ accessor
> > > > function or associated MEMBER variable. The property will be invalid.
> > > > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:18:
> > > > Warning: Property declaration calendar has no READ accessor function or
> > > > associated MEMBER variable. The property will 

Re: Submitting Kalendar for KDE Review

2021-10-18 Thread Albert Astals Cid
I think you may have dropped k-c-d from the CC, adding it back.


El dissabte, 16 d’octubre de 2021, a les 14:18:08 (CEST), Claudio Cambra va 
escriure:
> Hi Albert,
> 
> Thanks for your thorough review, and sorry about the late reply. We've been
> working on addressing these issues and have fixed them:
> 
> - Q_PROPERTY compilation warnings are now fixed
> - persistentIndex in the todomodel class are now QModelIndex
> - We now provide vendored copies of the components we require from Kirigami
> Addons (for now, until KA passes KDE Review)
> - We added KLocalizedString::setApplicationDomain("kalendar");
> - We now use standaloneMonthName for our month-year strings per your
> recommendation
> - We fixed the issue with valgrind you brought up
> 
> Additionally, we went ahead and addressed the user-facing issues you
> commented on (thanks!):
> - You should now get a pointing-hand cursor when hovering over links in the
> incidence information drawer
> - Clicking on empty space in the various views should now deselect the
> current incidence and close the incidence information drawer
> 
> Once again, thank you for taking the time to review Kalendar -- if you find
> anything else we should fix, we will be very happy to do so!

Good work!

Cheers,
  Albert

> 
> Clau
> 
> On Mon, Oct 11, 2021 at 12:01 AM Albert Astals Cid  wrote:
> 
> > El diumenge, 10 d’octubre de 2021, a les 22:45:06 (CEST), Claudio Cambra
> > va escriure:
> > > Hey all,
> > >
> > > We would like to move Kalendar to KDE review. We would also like to ask
> > if
> > > it would be possible to be release along with KDE Gear since the KDE
> > > PIM libraries don't have a stable ABI, and we need to make sure we
> > > remain compatible with them.
> > >
> > > For those who don't know, Kalendar is a new calendar application built
> > > with Kirigami and Akonadi. Our repo is located at
> > > https://invent.kde.org/pim/kalendar/. We welcome any feedback!
> >
> > These seem like important enough to fix
> >
> > AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/hourlyincidencemodel.h:30:
> > Warning: Property declaration model has no READ accessor function or
> > associated MEMBER variable. The property will be invalid.
> > AutoMoc:
> > /home/tsdgeos/devel/kde/kalendar/src/incidenceoccurrencemodel.h:42:
> > Warning: Property declaration filter has no READ accessor function or
> > associated MEMBER variable. The property will be invalid.
> > AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/multidayincidencemodel.h:35:
> > Warning: Property declaration model has no READ accessor function or
> > associated MEMBER variable. The property will be invalid.
> > AutoMoc:
> > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:17:
> > Warning: Property declaration incidenceChanger has no READ accessor
> > function or associated MEMBER variable. The property will be invalid.
> > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:18:
> > Warning: Property declaration calendar has no READ accessor function or
> > associated MEMBER variable. The property will be invalid.
> > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:19:
> > Warning: Property declaration filter has no READ accessor function or
> > associated MEMBER variable. The property will be invalid.
> >
> >
> >
> >
> > This is also interesting
> >
> > /home/tsdgeos/devel/kde/kalendar/src/todomodel.cpp:168:39: warning: loop
> > variable ‘persistentIndex’ of type ‘const QPersistentModelIndex&’ binds to
> > a temporary constructed from type ‘const QModelIndex’
> > [-Wrange-loop-construct]
> >   168 | for (const QPersistentModelIndex &persistentIndex :
> > persistentIndexes) {
> > m_persistentIndexes << persistentIndex; // Stuff we have to update
> > onLayoutChanged
> >
> >
> > You're pretending to keep a list of persistent model indexes, but you're
> > not, because m_persistentIndexes is a QModelIndexList that is a
> > QList so you're losing the "persistence" of it.
> >
> > So you either
> >
> >   A) don't need the persistance of it all if this works and
> > persistentIndex should just be a QModelIndex
> > or
> >   B) you should make m_persistentIndexes a list of QPersistentModelIndex
> > (and remove the & from the persistentIndex declaration to make the warning
> > go away).
> >
> >
> >
> > Aand I can't run it ^_^
> >
> > QQmlApplicationEngine failed 

Re: Submitting SubtitleComposer for KDE Review

2021-10-15 Thread Albert Astals Cid
I think you may have dropped k-c-d from the CC, adding it back.

El divendres, 15 d’octubre de 2021, a les 22:18:00 (CEST), Mladen Milinkovic va 
escriure:
> On 10/13/21 23:03, Albert Astals Cid wrote:
> > I think the tests are somehow not correctly flagged as tests, running ctest 
> > will only run the appstream check and src/tests/test-subtitle, but not 
> > test-core-rangelist and the rest.
> 
> Fixed in ec9ffba - I'm pretty sure this was working fine at some point in 
> (not so distant) past.
> 
> 
> > The first text format change doesn't seem to trigger the "file has changed 
> > and we should enable saving" logic. i.e. i have written a new subtitle line 
> > that says "HOLA" and saved the subtitle. Now if i select all the text and 
> > press the strikeout button, the save button does not get enabled, if i 
> > press the strikeout button again, the save button correctly gets enabled.
> 
> I believe this was happening sometimes due to "relatively scary valgrind 
> warning" below... looks like it's not happening 
> anymore - could you please confirm?

Seems to be working now :)

> 
> 
> > If i close a video while it's playing, the Play button will still be 
> > enabled (if i stop the video it will not)
> 
> Fixed in 663d209
> 
> 
> > Opening a .srt i just created and editing one of the subtitle lines i get 
> > this relatively scary valgrind warninghttps://ghostbin.com/YGnmL
> 
> Fixed in 663d209. QUndoStack::push(action) can merge and delete action, in 
> those cases it ended with invalid read 
> immediately afterwards.
> 
> 
> > When opening an existing .srt, there is a few Subtitle::insertLine calls 
> > that end up calling Subtitle::processAction with the if(app()->subtitle() 
> > != this) situation. I think all those Actions leak, because you just call 
> > redo on them but they are not deleted by anyone, no?
> 
> Yes they were leaking fixed them with 911b94b.
> 
> There are still some definite leaks after closing application:
>   - libfontconfig/QTextDocument (FcFontRenderPrepare 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=655989)
>   - QXcbGlxWindow::createVisual() calling radeon_dri.so and 
> amdgpu_winsys_create()
>   - Breeze::WidgetStateEngine::registerWidget calling QObject::connect (might 
> be related to 
> "QCoreApplication::postEvent: Unexpected null receiver" messages at 
> application shutdown)
>   - KF5WidgetAddons (KSelectActionPrivate::init())
> 
> There are also some memory errors that seem caused by KIO/KUrlRequester.
> 
> There are some possible leaks related to QTextDocument and rendering, will 
> investigate ASAP if it's due to something 
> that SC does wrong.

I think most of those are not your fault, but if you can spend a bit of time 
investigating won't hurt :)

> 
> > The "Report bug" incorrectly links 
> > tohttps://invent.kde.org/multimedia/subtitlecomposer/-/issues  instead of 
> > bug.kde.org
> 
> I didn't change the bug report url to bugs.kde.org yet as it doesn't seem 
> possible to file Subtitle Composer bugs there?
> Would prefer to change it right before SC gets included there if it's 
> necessary. There are SC binaries that get 
> generated pretty often that people are using - I'd like them to have a bug 
> report url they can use to report bugs.

Ok, then should open a request at 
https://phabricator.kde.org/maniphest/task/edit/form/2/ so that a 
subtitlecomposer product is created :)

Cheers,
  Albert

> 
> > 
> > Cheers,
> >Albert
> 
> Thank you!
> 






Re: Submitting SubtitleComposer for KDE Review

2021-10-13 Thread Albert Astals Cid
El dimecres, 13 d’octubre de 2021, a les 17:23:54 (CEST), Mladen Milinkovic va 
escriure:
> Hello everyone,
> 
> I would like to move subtitlecomposer to kdereview.
> 
> https://invent.kde.org/multimedia/subtitlecomposer
> 
> https://subtitlecomposer.kde.org/
> 
> Subtitle Composer is an app to create, sync, translate, ... text subtitles 
> for videos and media.

I think the tests are somehow not correctly flagged as tests, running ctest 
will only run the appstream check and src/tests/test-subtitle, but not 
test-core-rangelist and the rest.

The first text format change doesn't seem to trigger the "file has changed and 
we should enable saving" logic. i.e. i have written a new subtitle line that 
says "HOLA" and saved the subtitle. Now if i select all the text and press the 
strikeout button, the save button does not get enabled, if i press the 
strikeout button again, the save button correctly gets enabled.

If i close a video while it's playing, the Play button will still be enabled 
(if i stop the video it will not)

Opening a .srt i just created and editing one of the subtitle lines i get this 
relatively scary valgrind warning https://ghostbin.com/YGnmL

When opening an existing .srt, there is a few Subtitle::insertLine calls that 
end up calling Subtitle::processAction with the if(app()->subtitle() != this) 
situation. I think all those Actions leak, because you just call redo on them 
but they are not deleted by anyone, no?

The "Report bug" incorrectly links to 
https://invent.kde.org/multimedia/subtitlecomposer/-/issues instead of 
bug.kde.org

Cheers,
  Albert

> 
> Thanks
> 






Re: Submitting Kalendar for KDE Review

2021-10-10 Thread Albert Astals Cid
El diumenge, 10 d’octubre de 2021, a les 22:45:06 (CEST), Claudio Cambra va 
escriure:
> Hey all,
> 
> We would like to move Kalendar to KDE review. We would also like to ask if
> it would be possible to be release along with KDE Gear since the KDE
> PIM libraries don't have a stable ABI, and we need to make sure we
> remain compatible with them.
> 
> For those who don't know, Kalendar is a new calendar application built
> with Kirigami and Akonadi. Our repo is located at
> https://invent.kde.org/pim/kalendar/. We welcome any feedback!

These seem like important enough to fix

AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/hourlyincidencemodel.h:30: 
Warning: Property declaration model has no READ accessor function or associated 
MEMBER variable. The property will be invalid.
AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/incidenceoccurrencemodel.h:42: 
Warning: Property declaration filter has no READ accessor function or 
associated MEMBER variable. The property will be invalid.
AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/multidayincidencemodel.h:35: 
Warning: Property declaration model has no READ accessor function or associated 
MEMBER variable. The property will be invalid.
AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:17: 
Warning: Property declaration incidenceChanger has no READ accessor function or 
associated MEMBER variable. The property will be invalid.
/home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:18: Warning: 
Property declaration calendar has no READ accessor function or associated 
MEMBER variable. The property will be invalid.
/home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:19: Warning: 
Property declaration filter has no READ accessor function or associated MEMBER 
variable. The property will be invalid.




This is also interesting

/home/tsdgeos/devel/kde/kalendar/src/todomodel.cpp:168:39: warning: loop 
variable ‘persistentIndex’ of type ‘const QPersistentModelIndex&’ binds to a 
temporary constructed from type ‘const QModelIndex’ [-Wrange-loop-construct]
  168 | for (const QPersistentModelIndex &persistentIndex : 
persistentIndexes) {
m_persistentIndexes << persistentIndex; // Stuff we have to update 
onLayoutChanged


You're pretending to keep a list of persistent model indexes, but you're not, 
because m_persistentIndexes is a QModelIndexList that is a QList 
so you're losing the "persistence" of it.

So you either 

  A) don't need the persistance of it all if this works and persistentIndex 
should just be a QModelIndex
or
  B) you should make m_persistentIndexes a list of QPersistentModelIndex  (and 
remove the & from the persistentIndex declaration to make the warning go away).



Aand I can't run it ^_^

QQmlApplicationEngine failed to load component
qrc:/main.qml:663:9: Type TodoPage unavailable
qrc:/TodoPage.qml:168:17: Type TodoTreeView unavailable
qrc:/TodoTreeView.qml:9:1: module "org.kde.kirigamiaddons.treeview" is not 
installed

What provides org.kde.kirigamiaddons.treeview ?
Ok kirigami-addons, this is a bit "problematic" because it is still listed 
under unstable in our download section 
https://download.kde.org/unstable/kirigami-addons/0.2/
Having something stable depend on something unstable is not "good".



You're missing a KLocalizedString::setApplicationDomain("kalendar"); just after 
your QApplication


I get a
"0 instead of 1 arguments to message {Ends on %1} supplied before conversion."
when selecting one of my existing events


You can't use " " to get month and year because for some 
languages "" is translated as "of monthname" so that the typical DD  
 looks fine for them and you get "25 of Janaury of 2023", but then if you 
do " " you get "of monthname 2021" which is wrong, so my suggestion is 
do something like i18nc("%1 is month name, %2 is year", "%1 %2", 
Locale.standaloneMonthName(bla), blo));



Running with valgrind i got
==42992== Invalid read of size 16
==42992==at 0x142A6C69: ???
==42992==by 0x191F6C17: ???
==42992==  Address 0x191f6c1e is 30 bytes inside a block of size 42 alloc'd
==42992==at 0x483E7C5: malloc (vg_replace_malloc.c:380)
==42992==by 0x7B2E7D2: QArrayData::allocate(unsigned long, unsigned long, 
unsigned long, QFlags) (in 
/usr/lib/libQt5Core.so.5.15.2)
==42992==by 0x7BA967A: QString::fromLatin1_helper(char const*, int) (in 
/usr/lib/libQt5Core.so.5.15.2)
==42992==by 0x1C68FE: QString::QString(QLatin1String) (qstring.h:1067)
==42992==by 0x1F1886: AttendeeStatusModel::AttendeeStatusModel(QObject*) 
(attendeesmodel.cpp:33)
==42992==by 0x1F223C: AttendeesModel::AttendeesModel(QObject*, 
QSharedPointer) (attendeesmodel.cpp:78)
==42992==by 0x1E2F2E: IncidenceWrapper::IncidenceWrapper(QObject*) 
(incidencewrapper.cpp:15)
==42992==by 0x1BC3E8: 
QQmlPrivate::QQmlElement::QQmlElement() (qqmlprivate.h:139)
==42992==by 0x1BC435: void QQmlPrivate::createInto(void*) 
(qqmlprivate.h:166)
==42

Re: Towards Excellent Defect Management

2021-09-14 Thread Albert Astals Cid
El dimarts, 14 de setembre de 2021, a les 17:23:01 (CEST), Harald Sitter va 
escriure:
> For many years now I've been unhappy with both the quality and volume
> of crash reports we get for our software. The barrier for crash report
> submissions is incredibly high because we've never really had tech to
> help elevate "insufficient" reports to become sufficient, outside the
> client on which the crash occurred. Out of the very few people that
> might want to report a crash even fewer will get beyond the first set
> of questions from drkonqi, once they've managed they still have to
> fight with their distro for debug symbols and quite possibly lose, and
> even if they win there is a good chance the report will either get a
> "this isn't very useful. install more symbols" comment or get marked
> as dupe. Meanwhile we are spending our days looking at duplicated
> crashes, or finding the right blurb to copy paste to ask for a better
> trace, or try to find out why software crashes that hasn't actually
> crashed for a year because the bug had already been fixed in the
> meantime.
> 
> We are wasting our users' time. We are wasting our time. This waste
> needs to stop.
> 
> The good news is that we have all the technical building blocks to fix
> it today. In fact, it's even getting better in the future still. All
> it takes is a bit of code and a bit of flexibility on our part.
> 
> A while ago I started looking into improving the drkonqi experience.
> Specifically: submitting crash reports into a purpose built crash
> tracking system rather than a bug tracking system. The advantages are
> kind of obvious and ranging from server-side de-duplication to
> server-side retracing. I've spent many afternoons reading up on and
> poking demo instances of every somewhat suitable software I could
> find, and Sentry looks like the best option for what we need. It is
> practically free software as far as we are concerned, scales
> tremendously, has systems for server-side deduplication, server-side
> cross-distro/platform retracing (which might also help with some of
> the open questions of richer tracing for windows and android), data
> scrubbing (what with privacy concerns), client and server-side tags,
> can try to figure out when a crash first appeared if supplied with
> commit data, can track the quality of specific releases, when a given
> crash was fixed, health reports, performance tracking, warning rules,
> health report emails, ... I've been playing with it for a month and
> still find amazing new things!
> 
> One of the best things about Sentry is that it has native support for
> debuginfod, enabling us to get debug symbols directly from
> distributions, solving the entire cross-distro aspect of crash tracing
> in just about the neatest way possible. We get the (incomplete) trace
> with lots of metadata, and Senty then uses the metadata to resolve the
> symbols through the distros' debuginfod instances to give us a
> complete trace.
> 
> Even better: with relatively minor adjustments to drkonqi we could use
> it right now and get immediate advantage of server-side retracing! I
> already have a blob of prototype code for drkonqi that piggybacks
> Sentry submission onto the existing code such that we can have both
> bugzilla and Sentry.
> 
> I am proposing that we roll out a Sentry instance for testing so we
> can see if we want to fully embrace it.
> 
> You can get a general sense of the features at Sentry's demo instance
> https://sentry.io/demo/sandbox/
> Here's a code dump for drkonqi
> https://invent.kde.org/sitter/drkonqi/-/commits/work/sentry

The description of all the things it can do sound excellent :)

Cheers,
  Albert

> 
> HS
> 






Re: Towards Excellent Defect Management

2021-09-14 Thread Albert Astals Cid
El dimarts, 14 de setembre de 2021, a les 20:35:40 (CEST), Ben Cooksley va 
escriure:
> On Wed, Sep 15, 2021 at 5:35 AM Albert Astals Cid  wrote:
> 
> > El dimarts, 14 de setembre de 2021, a les 17:23:01 (CEST), Harald Sitter
> > va escriure:
> > > It is  practically free software as far as we are concerned
> >
> > I guess this means it's not actually Free Software?
> >
> 
> Please see https://open.sentry.io/licensing/
> 
> The tl;dr is that the license it is provided under (BSL 1.1) is not OSI
> approved due to the restriction on it's use by cloud vendors (ie. it has an
> anti-AWS clause).
> That restriction lapses after 36 months, at which point it is Apache 2.0
> compatible.
> 
> Based on what Harald has written earlier, it looks like Sentry is the only
> suitable game in town for what we need - the only question is whether we
> are happy to make use of BSL 1.1 licensed software given that we have
> traditionally only deployed 100% open source software to our systems (which
> is why we use Gitlab CE over Gitlab EE)

Oh i feel we already discussed this in the past, right?

Sorry for bringing it up again then :)

Cheers,
  Albert

> 
> 
> > Cheers,
> >   Albert
> >
> >
> >
> Cheers,
> Ben
> 






Re: Towards Excellent Defect Management

2021-09-14 Thread Albert Astals Cid
El dimarts, 14 de setembre de 2021, a les 17:23:01 (CEST), Harald Sitter va 
escriure:
> It is  practically free software as far as we are concerned

I guess this means it's not actually Free Software?

Cheers,
  Albert




Re: Croutons in kdereview

2021-08-29 Thread Albert Astals Cid
El diumenge, 29 d’agost de 2021, a les 23:30:23 (CEST), Janet Blackquill va 
escriure:
> They're there for the CamelCase headers generating macro, which
> doesn't seem to understand snake_case.h headers.

Have you tried fixing that in ECM?

Do git+symlinks even work on windows? Or we don't care for Windows particularly 
for this lib?

Cheers,
  Albert

> 
> -- Janet
> 
> Am So., 29. Aug. 2021 um 16:05 Uhr schrieb Albert Astals Cid :
> >
> > El diumenge, 29 d’agost de 2021, a les 5:10:04 (CEST), Janet Blackquill va 
> > escriure:
> > > Hello,
> > >
> > > https://invent.kde.org/libraries/croutons is in kdereview now
> > >
> > > Croutons is a library containing assorted functionality for dealing
> > > with asynchronous code in Qt, most notably a future type that can be
> > > passed into QML as a JavaScript Thennable (similarly to Qt IVI's
> > > PendingReply) and headers for C++20 coroutine integration for the
> > > Croutons Future types + some Qt types that make sense to co_await.
> > >
> > > The library is largely headers-only, sans the FutureBase, which has
> > > one (1) associated source file needing to be compiled into a binary.
> >
> > Why the symlinks in lib/ ?
> >
> > Cheers,
> >   Albert
> >
> > >
> > > -- Janet
> > >
> >
> >
> >
> >
> 






Re: Croutons in kdereview

2021-08-29 Thread Albert Astals Cid
El diumenge, 29 d’agost de 2021, a les 5:10:04 (CEST), Janet Blackquill va 
escriure:
> Hello,
> 
> https://invent.kde.org/libraries/croutons is in kdereview now
> 
> Croutons is a library containing assorted functionality for dealing
> with asynchronous code in Qt, most notably a future type that can be
> passed into QML as a JavaScript Thennable (similarly to Qt IVI's
> PendingReply) and headers for C++20 coroutine integration for the
> Croutons Future types + some Qt types that make sense to co_await.
> 
> The library is largely headers-only, sans the FutureBase, which has
> one (1) associated source file needing to be compiled into a binary.

Why the symlinks in lib/ ?

Cheers,
  Albert

> 
> -- Janet
> 






Re: Haruna in kdereview

2021-08-09 Thread Albert Astals Cid
El dilluns, 9 d’agost de 2021, a les 22:02:29 (CEST), George Florea Banus va 
escriure:
> On 09.08.2021 22:22, Albert Astals Cid wrote:
> > El dilluns, 9 d’agost de 2021, a les 16:23:27 (CEST), George Florea Banus 
> > va escriure:
> >> Or where to go from here.
> > Which are your doubts?
> 
> I'd like to get the review done so I can make a new release.
> 
> Also, after review, how do I version the app? Do I have to make a 1.0 
> release or can I make it 0.7.0?

That's all up to you :)

https://community.kde.org/ReleasingSoftware should have some guidelines.

Cheers,
  Albert

> 
> > Cheers,
> >Albert
> 






Re: Haruna in kdereview

2021-08-09 Thread Albert Astals Cid
El dilluns, 9 d’agost de 2021, a les 16:23:27 (CEST), George Florea Banus va 
escriure:
> Or where to go from here.

Which are your doubts?

Cheers,
  Albert

> 
> On 19.07.2021 15:54, George Florea Banus wrote:
> > Hello
> >
> > I would like to move Haruna to KDEReview.
> >
> > Haruna is a media player based on mpv, focus is on video playback.
> >
> > https://invent.kde.org/multimedia/haruna
> >
> 






Re: Skanpage moved to kdereview

2021-08-09 Thread Albert Astals Cid
El dilluns, 9 d’agost de 2021, a les 17:34:52 (CEST), Harald Sitter va escriure:
> it works amazingly well. good job! I mostly only have some nitpicky stuff
> 
> - since the code base is really close to
> complete reuse coverage, it might be nice to push it over the finishing
> line and then `reuse lint` it to not have it fall behind again
> - your cmake code is GPL but policy-wise it should be BSD-ish. any
> chance this can be relicensed?
> - for stylistic consistency with other repos you should probably use
> https://cmake.org/cmake/help/v3.16/module/FeatureSummary.html
> - testconfig.h.in -> I think you want
> https://doc.qt.io/qt-5/qtest.html#QFINDTESTDATA ;)
> - getUnitString -> can that maybe use
> https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html
> - the scanner options window is not high enough by default on my
> system (with Noto Sans 10pt as font)
> - the options really ought to use an apply/cancel pattern. applying
> changes on the fly feels very strange for a KDE app
> - something is wonky with the "Scan area size" label in the options
> window. it appears to be constructed from the string "Scan area size"
> and the string ":". they should be one string though. indeed the other
> options are one string :shrug:
> - not really a fan of the comboboxes and long options label in the
> toolbar. it feels very crowded and with long scanner names the window
> easily takes up more than half my screen width at minimum size
> - I'm not sure if that is the code's fault (it certainly looks
> correct) but in french the documentlist.qml bottom label reads `1
> page` when there are no pages

The bug is in the code, you can not assume that the "singular form" is for n == 
1.

In french the singular form is used for 1 and 0, so it needs to be %1 page and 
not "1 page".

It's tricky :/

Cheers,
  Albert

> 
> HS
> 






Re: Skanpage moved to kdereview

2021-08-08 Thread Albert Astals Cid
El diumenge, 8 d’agost de 2021, a les 18:50:17 (CEST), Alexander Stippich va 
escriure:
> Hello everyone,
> 
> Skanpage is now in kdereview. 
> Skanpage is a scanning application and originally a concept from Kåre Särs 
> from approx. 5 years ago. I started to work on it late last year and now 
> deemed it ready for a wider audience. 
> The main user-visible difference to the existing Skanlite is that it is 
> designed to be a dedicated multi-page scanning application with PDF export. 
> The main technical difference is that it is using QtQuick for its interface.
> 
> My plan is to directly ask for Skanpage to be included in KDE Gear if 
> possible 
> once it has passed kdereview.
> 
> Skanpage needs the 21.08 KDE Gear release of libksane. A lot of work has been 
> done with the help of Kåre to untangle the QWidget-based KSaneWidget into a 
> separate KSaneWidget and a logic interface named KSaneCore, where only 
> the latter is used by Skanpage. The new KSaneCore component is released with 
> 21.08. In the future, it is planned to spin off the KSaneCore component into 
> its own library without any QWidget-dependency.
> 
> Some features that are still missing from my point of view:
> - missing keyboard navigation
> - shortcut management
> - missing authentication for access-restricted devices
> - OCR (proof-of-concept available)
> - in the far future: a convergent interface for networked scanners on mobile 
> devices
> 
> There are tons of errors  thrown when reselecting a scanner device. This 
> seems 
> to be an issue with Kirigami.FormLayout and the bug is reported. Also, some 
> binding loops occur within the ScrollView, but the application still behaves.

Have you also reported the fact that disabled toolbar buttons look exactly like 
the enabled ones?

Albert

> 
> So while certainly some features are still missing, I find Skanpage to be 
> quite 
> useful already and an improvement over Skanlite for document scanning, 
> especially with ADF scanners.
> 
> Best regards,
> Alex
> 
> 
> 
> 






Re: Haruna in kdereview

2021-07-20 Thread Albert Astals Cid
El dimarts, 20 de juliol de 2021, a les 8:16:58 (CEST), George Florea Banus va 
escriure:
> > The thing you call Header in View -> "Hide header" is called 
> > historically called toolbar, i think you should rename it.
> 
> I'll called it header because that's what Qt calls it and the footer is 
> also a toolbar.
> Do you want me to change only user facing part or  everything? What 
> about the footer?

User facing part is enough.

Typically one would call the footer status bar, but that one is so full of 
widgets that i don't think statusbar would be a good name, anyhow the name is 
not exposed to the user, right?

Cheers,
  Albert

> 
> > All in all, the app is nice, but IMHO suffers a bit from being QML 
> > based and not being able to use KXMLGUI.
> >
> > Cheers,
> >Albert
> 
> Thanks for the review.
> 
> 
> 






Re: Haruna in kdereview

2021-07-19 Thread Albert Astals Cid
El dilluns, 19 de juliol de 2021, a les 14:54:26 (CEST), George Florea Banus va 
escriure:
> Hello
> 
> I would like to move Haruna to KDEReview.
> 
> Haruna is a media player based on mpv, focus is on video playback.
> 
> https://invent.kde.org/multimedia/haruna


You need a

KLocalizedString::setApplicationDomain("haruna") 

just after creating your QApplication. Otherwise translations are not going to 
be loaded.

The menus look&feel is a bit broken (separators and highlight being drawn past 
the actual menu width, icons being painted "blurrily"), but i guess that's not 
really your fault.

The Setting has a "Help!" <- remove the !

in that "Help!" menu you have a KHelpCenter entry that should be called "Haruna 
handbook" like in the rest of applications.

The first time i open the app the "Color scheme" combo box doesn't have 
contents, given there's a default entry i'd expect to see that.

The "english only" help is kind of weird. Why are you using html for that? 
Seems like something you should do with tooltips and if those QML components 
don't support tooltips, at least make the help page with QML so it's 
translatable?

I've no idea what the "Open config..." entries in the settings menu are 
supposed to do, but both fail with "file doesn't exist" for me.

Esc not only exits fullscreen (good) but enters fullscreen (weird).

The "Primary Subtitle" MenuItem that tries to be a title is kind of weird 
because it has exactly the same look&feel that the other menus, at least 
disable clicking on it, because i can click on it and then it's even more 
confusing if it's really wants to be a title or not without reading the code :D

It does remember the last thing i was playing, which I dislike, I tried to look 
for a config option to disable it but doesn't seem to be one. Feel free to 
ignore this.

The thing you call Header in View -> "Hide header" is called historically 
called toolbar, i think you should rename it.

All in all, the app is nice, but IMHO suffers a bit from being QML based and 
not being able to use KXMLGUI.

Cheers,
  Albert




Re: KHealthCertificate in kdereview

2021-07-11 Thread Albert Astals Cid
El diumenge, 11 de juliol de 2021, a les 15:32:27 (CEST), Volker Krause va 
escriure:
> Hi,
> 
> KHealthCertificate is a library for decoding digital vaccination, test and 
> recovery certificates. Supported formats/features are:
> * EU DGC: almost all data found in vaccination, test and recovery 
> certificates, verification of ECDSA and RSA/PSS signatures.
> * India: basic support for vaccination certificates, no signature 
> verification 
> yet.
> 
> https://invent.kde.org/pim/khealthcertificate
> 
> If you have more samples/information about the Indian system, or systems 
> deployed elsewhere in the world, I'd be very much interested :)
> 
> The first user of this is the basic vaccination certificate manager in 
> Itinerary (currently optional), other potential users who have expressed 
> interest are Qrca and MyGnuHealth, as well as a possible stand-alone 
> vaccination certificate wallet app for Plasma Mobile.
> 
> Beyond kdereview the goal would be to join an automated release process, 
> Plasmo Gear is probably the best option of those given the expected users.
> 
> A note on translations: The only user-visible strings right now are those in 
> the EU DGC JSON files mapping various codes to human readable texts. Those 
> datasets are supposed to be translated eventually, by upstream. This however 
> isn't implemented yet apparently, the official apps are waiting for this as 
> well (e.g. https://github.com/Digitaler-Impfnachweis/covpass-android/blob/
> main/covpass-sdk/src/main/java/de/rki/covpass/sdk/storage/
> EUValueSetRepository.kt#L11). So I'd rather wait for that getting fixed 
> rather 
> than bothering our translators with various medical product names :)
> 
> For testing this you need a fairly recent KF5, KCodecs 5.84 for Base45 
> support 
> and Prison 5.85 if your phone screen is too small for the usually very large 
> barcodes.

Rename some non installed headers in src/lib to *_p.h ?

I tried reading the headers to see how i would use this and i was left with 
some questionmarks.

Am I supposed to call KHealthCertificateParser::parse? With what? Because it 
says "data received from a barcode scanner" but i guess that's not the QImage 
data?

Also it returns a QVariant ? What's in there?

Cheers,
  Albert 

> 
> Thanks,
> Volker






Re: kpeoplevcard in kdereview

2021-06-14 Thread Albert Astals Cid
El dissabte, 12 de juny de 2021, a les 15:56:36 (CEST), Nicolas Fella va 
escriure:
> Hi,
> 
> https://invent.kde.org/pim/kpeoplevcard is now in kdereview.
> 
> kpeoplevcard is a data source plugin for KPeople that provides contacts
> based on VCard files on the disk. The 0.1 release has been in use in
> plasma-phonebook and kdeconnect-sms for a while, but I just realized it
> was never properly reviewed.
> 
> I did some cleanup work to bring the repo up to standards, but in
> general the thing is rather "finished". There is a pending MR adding
> full REUSE compliance waiting for approval from the copyright holders.

There's i18n but no Messages.sh nor -DTRANSLATION_DOMAIN= (assuming this is a 
lib or similar and it makes sense to use -DTRANSLATION_DOMAIN=)

Cheers,
  Albert

> 
> Cheers
> 
> Nico
> 
> 






Re: Qt5PatchCollection versions (broken QWE)

2021-05-27 Thread Albert Astals Cid
El dijous, 27 de maig de 2021, a les 6:10:47 (CEST), Harald Sitter va escriure:
> On Wed, May 26, 2021 at 7:39 PM Antonio Rojas  wrote:
> >
> > El miércoles, 26 de mayo de 2021 13:19:34 (CEST), Harald Sitter escribió:
> > > Hola
> > >
> > > QWE and QtScript in our kde/5.15 branches currently have unaligned 
> > > versions.
> > >
> > > QWE 5.15.4:
> > > https://invent.kde.org/qt/qt/qtwebengine/-/blob/kde/5.15/.qmake.conf
> > >
> > > QtBase 5.15.3:
> > > https://invent.kde.org/qt/qt/qtbase/-/blob/kde/5.15/.qmake.conf
> > >
> > > This then breaks version alignment expectations. For example QWECore
> > > requires QtWebChannel at the same version but since the versions are
> > > misaligned the requirement check changes.
> >
> > This is because the upstream QWE and QtScripts 5.15 branches are public,
> > unlike the other Qt repos. The versioning issue is discussed here:
> >
> > https://www.qt.io/blog/building-qt-webengine-against-other-qt-versions
> >
> > Lowering the version number would be misleading since this is the real,
> > upstream 5.15 code, which is post-5.15.4 already.
> 
> I suppose then we need to fix the cmake check?

That would be one of the patches that would make some sense even if not coming 
from upstream i guess.

Cheers,
  Albert

> 
> Right now building the lot of our patch collection branches leads to
> skrooge not being able to build because of that defect. That seems
> silly at the best of times.
> 
> HS
> 






Re: New repo in kdereview: kasts

2021-05-27 Thread Albert Astals Cid
El dijous, 27 de maig de 2021, a les 12:20:20 (CEST), Bart De Vries va escriure:
> Hello everyone!
> 
> I would like to move kasts to kdereview.
> 
> https://invent.kde.org/plasma-mobile/kasts  
> 
> 
> Kasts is a podcast app for Plasma Mobile. It was started as a fork of 
> Alligator, the Plasma Mobile feed reader. It currently features a play queue, 
> play position resume/persistence, MPRIS2 support, and suspend inhibition.

The left bar is super confusing on the desktop, the 4 first elements are 
"toggles" but Settings and About are not, so if you do About, Settings, About, 
Settings, About, Settings, About, Settings, Download.

You don't end up in Download, you're still in Settings, and what's worse, now 
you have to press back like 10 times to remove all those About and Settings 
pages that are on the stack.

Import podcast defaulting to Planet KDE is a bit weird?

text: (!isQueue && entry.queueStatus ? "·  " : "") + 
entry.updated.toLocaleDateString(Qt.locale(), Locale.NarrowFormat) + 
(entry.enclosure ? ( entry.enclosure.size !== 0 ? "  ·  " + 
Math.floor(entry.enclosure.size / (1024 * 1024)) + "MB" : "") : "" )

is a huge text puzzle that needs to be an i18nc call (or many if needed) so 
translators can rearrange things in the order that make sense in their language 
+ translate MB

Cheers,
  Albert

> 
> Kind regards,
> Bart De Vries
> 
> 






Re: New repo in kdereview: kalk

2021-05-03 Thread Albert Astals Cid
El dilluns, 3 de maig de 2021, a les 0:49:37 (CEST), Albert Astals Cid va 
escriure:
> El diumenge, 2 de maig de 2021, a les 8:43:42 (CEST), hanyoung va escriure:
> > Change the default output to float, however I'm not sure how to add 
> > autotest. I want to unit test InputManager but it's not a library.
> 
> I wrote you an autotest 
> https://invent.kde.org/plasma-mobile/kalk/-/merge_requests/21
> 
> and while writing the autotest i realized why kalk is so broken for me.
> 
> You don't take into account there are locales that use , as a decimal 
> separator instead of .
> 
> So when i do 70/9*9 in English, it works, if i do it in Catalan it does not 
> because something gets confused because the result from 70/9 is not 7.77 
> but 7,77
> 
> Sadly the autotest doesn't help catch this because i force the C locale in 
> it, will think how it can be improved to maybe catch that tomorrow.

Added a failing test at 
https://invent.kde.org/plasma-mobile/kalk/-/merge_requests/22

Now you have to make it work :)

Cheers,
  Albert

> 
> Cheers,
>   Albert
> 
> > 
> > ‐‐‐ Original Message ‐‐‐
> > On Sunday, May 2, 2021 6:04 AM, Albert Astals Cid  wrote:
> > 
> > > El dissabte, 1 de maig de 2021, a les 7:26:24 (CEST), hanyoung va 
> > > escriure:
> > >
> > > > I've fixed the keyboard bug and change the division result between two 
> > > > integers to float.
> > >
> > > I don't think that changing knumber to do that is a great idea, keeping 
> > > integer divisions as fraction instead of float seems the correct thing to 
> > > do, you should instead change the ui to be able to show fractions as 
> > > floats but to keep the internal representation as good as possible, i.e. 
> > > as a fraction.
> > >
> > > Not sure if related to that or not, but now if i type
> > > 65 / 9 = * 7 =
> > >
> > > i get 1.3e+16 which doesn't seem correct, should be 50,...
> > >
> > > I'd really suggest you add autotests for this.
> > >
> > > Cheers,
> > > Albert
> > 
> 
> 
> 
> 
> 






Re: Koko in KDEReview

2021-05-03 Thread Albert Astals Cid
El dimarts, 4 de maig de 2021, a les 0:36:57 (CEST), Carl Schwan va escriure:
> Le mardi, mai 4, 2021 12:21 AM, Albert Astals Cid  a écrit :
> 
> > El dimarts, 4 de maig de 2021, a les 0:07:19 (CEST), Carl Schwan va 
> > escriure:
> >
> > > Le lundi, mai 3, 2021 4:35 PM, Harald Sitter sit...@kde.org a écrit :
> > >
> > > > On 23.04.21 01:00, Carl Schwan wrote:
> > > >
> > > > > > > > > -   please get a bugzilla produce created for it
> > > > > > >
> > > > > > > Not a fan of that. This will ends up exactly like the www bugs, 
> > > > > > > something
> > > > > > > that I look into every 6 months. We already have many issues 
> > > > > > > opened in
> > > > > > > invent and it's working fine for us.
> > > > > >
> > > > > > Did I miss something? The last agreement I recall was that we are 
> > > > > > using
> > > > > > bugzilla for bugs and gitlab for tasks for the time being. If even 
> > > > > > as
> > > > > > developer I have to go hunting where $project tracks their bugs I'm 
> > > > > > sure
> > > > > > not going to be a happy camper.
> > > > > >
> > > > > > > Also we don't use KCrash.
> > > > > >
> > > > > > Shouldn't you?
> > > > >
> > > > > The problem is that DrKonqi doesn't really work on Plasma Mobile and 
> > > > > I don't think
> > > > > this justify getting locked up in using Bugzilla. If having a 
> > > > > bugzilla product
> > > > > is really required, we can request one but I can't guarantee that I 
> > > > > will look at it
> > > > > more often than I look at the kde-www bug reports in bugzilla (every 
> > > > > 6 months).
> > > >
> > > > Let's consider it required then.
> > > > If you don't care about crash reports that's your choice I guess, but it
> > > > does kinda call into question the product quality since you also don't
> > > > have an alternative system to the kcrash-drkonqi-bugzilla caravan. Quite
> > > > clearly koko would have benefited from crash tracking and since the only
> > > > solution we presently have for that is the aforementioned stack it quite
> > > > clearly also would have benefited from being on bugzilla to receive
> > > > those crash reports and potentially move to other products if the crash
> > > > is not in koko. After all, crashes get submitted to the product that
> > > > crashed, not the library of the top most frame.
> > > > I have to be honest, it is a bit surreal to even have to argue this. I
> > > > get thorough enjoyment out of throwing tomatoes at the current system
> > > > but to actively pretend that the problem-domain of crashing software
> > > > doesn't exist so you don't have to look at bugzilla sure as heck doesn't
> > > > solve anything. In particular since you pitched koko as convergent and
> > > > useful on the desktop.
> > > > If all that's stopping you from embracing crash tracking is drkonqi then
> > > > I am happy to tell you that sticking a mobile-suitable UI on top
> > > > shouldn't be all that difficult ;)
> > >
> > > Would you be open to an MR adding GitLab support to DrKonqi?
> >
> > We don't use gitlab for user bug reports.
> 
> Do you have a link to the policy? I looked into 
> https://community.kde.org/Policies
> and I found nothing. https://manifesto.kde.org/commitments.html says that we
> should only use online service hosted by KDE, but as far I know invent is 
> hosted by
> KDE.

"The project stays true to established practices"

The established practice in KDE is clearly using bugzilla for bugs. 

Cheers,
  Albert

> 
> Also considering that bugzilla is almost unmaintained, that should be 
> something
> to reconsider if there is really such policy. For info, the Bugzilla UX 
> initiative
> died when the lead developer was fired by Mozilla. And the Harmony project 
> which is
> trying to bring back the improvements from BMO (the Mozilla internal fork) is
> progressing at an abysmal pace. Bugzilla itself had its last code 
> contribution one year
> ago.
> 
> Cheers,
> Carl
> 
> >
> > Cheers,
> > Albert
> >
> > > > HS
> 
> 
> 






Re: Koko in KDEReview

2021-05-03 Thread Albert Astals Cid
El dimarts, 4 de maig de 2021, a les 0:07:19 (CEST), Carl Schwan va escriure:
> Le lundi, mai 3, 2021 4:35 PM, Harald Sitter  a écrit :
> 
> > On 23.04.21 01:00, Carl Schwan wrote:
> >
> > > > > > > -   please get a bugzilla produce created for it
> > > > >
> > > > > Not a fan of that. This will ends up exactly like the www bugs, 
> > > > > something
> > > > > that I look into every 6 months. We already have many issues opened in
> > > > > invent and it's working fine for us.
> > > >
> > > > Did I miss something? The last agreement I recall was that we are using
> > > > bugzilla for bugs and gitlab for tasks for the time being. If even as
> > > > developer I have to go hunting where $project tracks their bugs I'm sure
> > > > not going to be a happy camper.
> > > >
> > > > > Also we don't use KCrash.
> > > >
> > > > Shouldn't you?
> > >
> > > The problem is that DrKonqi doesn't really work on Plasma Mobile and I 
> > > don't think
> > > this justify getting locked up in using Bugzilla. If having a bugzilla 
> > > product
> > > is really required, we can request one but I can't guarantee that I will 
> > > look at it
> > > more often than I look at the kde-www bug reports in bugzilla (every 6 
> > > months).
> >
> > Let's consider it required then.
> >
> > If you don't care about crash reports that's your choice I guess, but it
> > does kinda call into question the product quality since you also don't
> > have an alternative system to the kcrash-drkonqi-bugzilla caravan. Quite
> > clearly koko would have benefited from crash tracking and since the only
> > solution we presently have for that is the aforementioned stack it quite
> > clearly also would have benefited from being on bugzilla to receive
> > those crash reports and potentially move to other products if the crash
> > is not in koko. After all, crashes get submitted to the product that
> > crashed, not the library of the top most frame.
> >
> > I have to be honest, it is a bit surreal to even have to argue this. I
> > get thorough enjoyment out of throwing tomatoes at the current system
> > but to actively pretend that the problem-domain of crashing software
> > doesn't exist so you don't have to look at bugzilla sure as heck doesn't
> > solve anything. In particular since you pitched koko as convergent and
> > useful on the desktop.
> >
> > If all that's stopping you from embracing crash tracking is drkonqi then
> > I am happy to tell you that sticking a mobile-suitable UI on top
> > shouldn't be all that difficult ;)
> 
> Would you be open to an MR adding GitLab support to DrKonqi?

We don't use gitlab for user bug reports.

Cheers,
  Albert

> 
> > HS
> 
> 
> 






  1   2   3   4   5   6   7   8   9   10   >