Short review of two commits
Hi I fixed a bug in amarok. Change is quite simple but i changed some methodnames and class members. Since i am not that firm in all things amarok could someone please review if the commit 121f91675dbf68072cefe486ccdabfff922cc0d5 and the one before that. is bc and source compatible? Does Amarok have such promises? Perhaps i am just to kdelibs style sensible. Mike ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Localized icons in Amarok
8 января 2011 г. 20:47 пользователь Sergey Ivanov <123k...@gmail.com> написал: > 2011/1/8 Alexander Potashev : >> Hi, >> >> I tried to make Amarok use Russian version of the icon >> filename-sample-rate.png. I've put that file here: >> /usr/share/apps/amarok/icons/hicolor/48x48/actions/l10n/ru/filename-sample-rate.png >> , but Amarok still uses the original icon (when running with Russian >> localization enabled). >> >> Is it possible to use localized versions of icons in Amarok? >> >> >> -- >> Alexander Potashev >> >> ___ >> Amarok-devel mailing list >> Amarok-devel@kde.org >> https://mail.kde.org/mailman/listinfo/amarok-devel >> >> > > IMHO, Hz looks much more better then Гц. +1 to Hz. I don't think that it will be a problem to have "Hz", which is international. -- Best regards, Valentyn Pavliuchenko ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Localized icons in Amarok
2011/1/8 Alexander Potashev : > Is it possible to use localized versions of icons in Amarok? Localized icon works for me now in Amarok, I needed to clean the icon cache properly. I'll start a discussion in Russian KDE mailing list about relevance of the localized icon. -- Alexander Potashev ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Localized icons in Amarok
2011/1/8 Thomas Lübking :
> (but then there could be just i18n("Hz") and no icon at all, yesno?)
Plain text "Hz" wouldn't look as an icon. Also, your solution is very
partial, you won't be able to localize the "bold"/"italic"/... icons
in KOffice/Calligra.
--
Alexander Potashev
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Localized icons in Amarok
Am Saturday 08 January 2011 schrieb Sergey Ivanov:
> > Is it possible to use localized versions of icons in Amarok?
No, but KDE should support *local* icons, ie. place them in
~/.kde/share/apps/amarok/icons/hicolor/48x48/actions/filename-sample-rate.png
- or so... =\
(but then there could be just i18n("Hz") and no icon at all, yesno?)
Thomas
___
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
Apologies
Hi, I was backing up my gmail with fetchmail and it seems something went horribly wrong. If this ML or any of you have got bounces of old mails from me, I'm really sorry about that. Nikhil ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Localized icons in Amarok
2011/1/8 Alexander Potashev : > Hi, > > I tried to make Amarok use Russian version of the icon > filename-sample-rate.png. I've put that file here: > /usr/share/apps/amarok/icons/hicolor/48x48/actions/l10n/ru/filename-sample-rate.png > , but Amarok still uses the original icon (when running with Russian > localization enabled). > > Is it possible to use localized versions of icons in Amarok? > > > -- > Alexander Potashev > > ___ > Amarok-devel mailing list > Amarok-devel@kde.org > https://mail.kde.org/mailman/listinfo/amarok-devel > > IMHO, Hz looks much more better then Гц. -- Sergey Ivanov ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
[Amarok] 137de44 Fix major issue with the Collection missing whole
commit 137de448c030cbfeb9de3bc21969f259a7291eca
branch master
Author: Mark Kretschmann
Date: Sat Jan 8 19:08:55 2011 +0100
Fix major issue with the Collection missing whole albums, and mixing up
artists.
Big thanks to Ralf Engels, who came up with this fix! :)
BUG: 262459
CCMAIL: amarok-devel@kde.org
diff --git a/src/core-impl/collections/db/sql/SqlScanResultProcessor.cpp
b/src/core-impl/collections/db/sql/SqlScanResultProcessor.cpp
index d861da8..4a2b1ab 100644
--- a/src/core-impl/collections/db/sql/SqlScanResultProcessor.cpp
+++ b/src/core-impl/collections/db/sql/SqlScanResultProcessor.cpp
@@ -161,6 +161,7 @@ SqlScanResultProcessor::commitTrack(
CollectionScanner::Track *track,
if( m_foundTracks.contains( uid ) )
{
warning() << "track"
Localized icons in Amarok
Hi, I tried to make Amarok use Russian version of the icon filename-sample-rate.png. I've put that file here: /usr/share/apps/amarok/icons/hicolor/48x48/actions/l10n/ru/filename-sample-rate.png , but Amarok still uses the original icon (when running with Russian localization enabled). Is it possible to use localized versions of icons in Amarok? -- Alexander Potashev <><>___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: This time of the year again: Splash Screen
On Sat, Jan 8, 2011 at 8:41 PM, Mark Kretschmann wrote: > > Nikhil, could you please put the original PNG image in Git Master too? > > It's good to have access to the original. Also, I would like to resize > it a bit, the image is very huge. > done. ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Reduce code duplication (combo box list)
> On Jan. 8, 2011, 3:12 p.m., Mark Kretschmann wrote: > > Yeah, you can do that. However, I have not actually tested your patch. Does > > it change anything in the behavior of the widget? I compiled Amarok with the first version of this patch (with «""»), and I haven't noticed any changes. - Alexander --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100309/#review775 --- On Jan. 6, 2011, 8:27 p.m., Alexander Potashev wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100309/ > --- > > (Updated Jan. 6, 2011, 8:27 p.m.) > > > Review request for Amarok. > > > Summary > --- > > Reduce code duplication when filling in items of combo box. > > > Diffs > - > > src/widgets/MetaQueryWidget.cpp 8868106 > > Diff: http://git.reviewboard.kde.org/r/100309/diff > > > Testing > --- > > > Thanks, > > Alexander > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Reduce code duplication (combo box list)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100309/#review775 --- Yeah, you can do that. However, I have not actually tested your patch. Does it change anything in the behavior of the widget? - Mark On Jan. 6, 2011, 8:27 p.m., Alexander Potashev wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100309/ > --- > > (Updated Jan. 6, 2011, 8:27 p.m.) > > > Review request for Amarok. > > > Summary > --- > > Reduce code duplication when filling in items of combo box. > > > Diffs > - > > src/widgets/MetaQueryWidget.cpp 8868106 > > Diff: http://git.reviewboard.kde.org/r/100309/diff > > > Testing > --- > > > Thanks, > > Alexander > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: This time of the year again: Splash Screen
On Wed, Jan 5, 2011 at 10:13 AM, Nikhil Marathe wrote: > On Wed, Jan 5, 2011 at 2:24 PM, Mark Kretschmann wrote: >>> Its a PNG, the earlier was a JPG, so is >>> that an issue >>> or can I modify App.cpp to simply use a PNG? >> >> Yes, you could do that. Or, if the PNG is very large in size, convert >> it to JPEG (but with good quality), and then put both in Git Master, >> so that we still have the original file available. > > Converted to JPEG >> >>> I suppose I also add Tomasz to the contributors/thanks to list as >>> splash screen artist right? >> > As Nightrose suggested, this will be done after the tagging so as not > to cause i18n issues. Nikhil, could you please put the original PNG image in Git Master too? It's good to have access to the original. Also, I would like to resize it a bit, the image is very huge. -- Mark Kretschmann Amarok Developer, CEO of Kretschmann Software Consulting Fellow of the Free Software Foundation Europe http://amarok.kde.org - http://fsfe.org - http://kde.org ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Reduce code duplication (combo box list)
> On Jan. 8, 2011, 8:42 a.m., Mark Kretschmann wrote: > > Looks good to me. At first I thought, this introduces string changes, we > > cannot do it before 2.4.0 release. But then I realized that you have just > > removed strings, but not changed any. > > > > One nitpick: You should probably change "" to QString(). May I commit this patch myself to the repository? (after changing "" to QString()) I hope that I have write access to the repo. At least, I cloned it via g...@git.kde.org:amarok.git. - Alexander --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100309/#review770 --- On Jan. 6, 2011, 8:27 p.m., Alexander Potashev wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100309/ > --- > > (Updated Jan. 6, 2011, 8:27 p.m.) > > > Review request for Amarok. > > > Summary > --- > > Reduce code duplication when filling in items of combo box. > > > Diffs > - > > src/widgets/MetaQueryWidget.cpp 8868106 > > Diff: http://git.reviewboard.kde.org/r/100309/diff > > > Testing > --- > > > Thanks, > > Alexander > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Regression Alert: Collection completely borked
On Sat, Jan 8, 2011 at 12:59 PM, Mark Kretschmann wrote: > On Fri, Jan 7, 2011 at 5:26 PM, Leo Franchi wrote: >> On Fri, Jan 7, 2011 at 11:16 AM, Mark Kretschmann >> wrote: >>> Ahoy, >>> >>> I just noticed that Amarok would play an album, say from Artist X, and >>> in reality it plays an album from Artist Y. Then, complete albums are >>> missing. >>> >>> I did a full rescan, it did not help. Then I nuked my database, but >>> the problem still persists: Genesis is now sounding a lot like Sting. >>> >>> Some debug output: >>> >>> amarok: [WARNING] [SqlRegistryP] Insert failed. >>> amarok: [ERROR__] [MySqlStorage] "GREPME MySQLe query failed! >>> (1062) Duplicate entry '0' for key 'statistics_url' on INSERT INTO >>> statistics (url,createdate,accessdate,score,rating,playcount,deleted) >>> VALUES >>> (NULL,NULL,NULL,NULL,0,0,0),(1,NULL,NULL,NULL,0,0,0),(2,NULL,NULL,NULL,0,0,0),(3,NULL,NULL,NULL,0,0,0),(4,NULL,NULL,NULL,0,0,0),(5,NULL,NULL,NULL,0,0,0),(6,NULL,NULL,NULL,0,0,0),(7,NULL,NULL,NULL,0,0,0),(8,NULL,NULL,NULL,0,0,0),(9,NULL, >>> NULL,NULL,0,0,0),(10,NULL,NU >>> >>> >>> What's going on there? >> >> Not sure. FWIW, my collection works fine here, both current and after >> a rm -rf and clean rescan. > > Ok, it seems the mistake was really on my side, or rather on Kubuntu's side: > > I did have a distro package of Amarok installed (no idea why), and my > Amarok picked up some wrong plugins. Bleh. Unfortunately, this was a red herring. The bug is for real :( -- Mark Kretschmann Amarok Developer, CEO of Kretschmann Software Consulting Fellow of the Free Software Foundation Europe http://amarok.kde.org - http://fsfe.org - http://kde.org ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Fix Inconsistencies with Organize Files Dialog when canceling Dialog
> On Jan. 8, 2011, 11:43 a.m., Sergey Ivanov wrote: > > I think that It's a bad idea to apply any settings when user press Cancel, > > so storing Format Presets should stay in onAccept slot. > > Agree with second statement. > > > > And finally this patch has nothing in common with mentioned Bug Report. That's the thing. Pressing "Save preset" gives the impression that the preset has indeed been saved. It has happened to me a couple of times that I created a new Preset, checked the preview, found something wrong with my tagging, pressed cancel, corrected the tags and then wanted to use the saved preset again which was gone... The way I see it both ways are essentially wrong from a certain Design perspective. I am by no means a GUI or Usability expert (my expertise comes down to 3 lectures regarding User Interface Design and Consistency in the user interface and some small Project in University). But what I learned is that you should make Dialogs as unambiguous as possible and with respect to that the discrepancy between the *strong* implication of the preset being "Saved" and then again not when pressing cancel is very hard to resolve. A solution to that would be to add a third button like "Do not move files but save settings" (Better wording needed :D) or to remove the cancel button altogether and remove it with something like "Close" that does not imply that the settings are discarded and save them anyway... Of the two the first option would probably be preferable as it doesn't remove the Deesktop Paradigma that Dialogs should have a cancel button that aborts everything it does. With respect to the bugreport: You're right, this review request has nothing to do with it, but could you then please comment on the bug why you reopened it? When I was checking in IRC with the person who apparently initiated that I found all these inconsistencies but not that the bug was still there. - Philipp --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100321/#review771 --- On Jan. 8, 2011, 12:12 p.m., Philipp Schmidt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100321/ > --- > > (Updated Jan. 8, 2011, 12:12 p.m.) > > > Review request for Amarok. > > > Summary > --- > > Fixes two errors: > > First: Presets are being saved explicitely, meaning they should persist even > when the Dialog is aborted/canceled. > Second: The state of the Current Collection Directory is saved regardless of > whether the Dialog was accepted or canceled. IMO it should only be saved like > all other values when it is accepted. > > > Diffs > - > > ChangeLog 3c337d1 > src/dialogs/OrganizeCollectionDialog.cpp b7d7850 > > Diff: http://git.reviewboard.kde.org/r/100321/diff > > > Testing > --- > > > Thanks, > > Philipp > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Fix Inconsistencies with Organize Files Dialog when canceling Dialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100321/ --- (Updated Jan. 8, 2011, 12:12 p.m.) Review request for Amarok. Changes --- Bugreference removed as it was incorrect. Summary --- Fixes two errors: First: Presets are being saved explicitely, meaning they should persist even when the Dialog is aborted/canceled. Second: The state of the Current Collection Directory is saved regardless of whether the Dialog was accepted or canceled. IMO it should only be saved like all other values when it is accepted. Diffs - ChangeLog 3c337d1 src/dialogs/OrganizeCollectionDialog.cpp b7d7850 Diff: http://git.reviewboard.kde.org/r/100321/diff Testing --- Thanks, Philipp ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Regression Alert: Collection completely borked
On Fri, Jan 7, 2011 at 5:26 PM, Leo Franchi wrote: > On Fri, Jan 7, 2011 at 11:16 AM, Mark Kretschmann wrote: >> Ahoy, >> >> I just noticed that Amarok would play an album, say from Artist X, and >> in reality it plays an album from Artist Y. Then, complete albums are >> missing. >> >> I did a full rescan, it did not help. Then I nuked my database, but >> the problem still persists: Genesis is now sounding a lot like Sting. >> >> Some debug output: >> >> amarok: [WARNING] [SqlRegistryP] Insert failed. >> amarok: [ERROR__] [MySqlStorage] "GREPME MySQLe query failed! >> (1062) Duplicate entry '0' for key 'statistics_url' on INSERT INTO >> statistics (url,createdate,accessdate,score,rating,playcount,deleted) >> VALUES >> (NULL,NULL,NULL,NULL,0,0,0),(1,NULL,NULL,NULL,0,0,0),(2,NULL,NULL,NULL,0,0,0),(3,NULL,NULL,NULL,0,0,0),(4,NULL,NULL,NULL,0,0,0),(5,NULL,NULL,NULL,0,0,0),(6,NULL,NULL,NULL,0,0,0),(7,NULL,NULL,NULL,0,0,0),(8,NULL,NULL,NULL,0,0,0),(9,NULL, >> NULL,NULL,0,0,0),(10,NULL,NU >> >> >> What's going on there? > > Not sure. FWIW, my collection works fine here, both current and after > a rm -rf and clean rescan. Ok, it seems the mistake was really on my side, or rather on Kubuntu's side: I did have a distro package of Amarok installed (no idea why), and my Amarok picked up some wrong plugins. Bleh. -- Mark Kretschmann Amarok Developer, CEO of Kretschmann Software Consulting Fellow of the Free Software Foundation Europe http://amarok.kde.org - http://fsfe.org - http://kde.org ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
tagging delay
Heya folks, given the remaining problems we are seeing I'm delaying tagging and release by 3 days. Please everyone have a look at the bugs tagged release_blocker and regression in bugzilla. There are saved searches linked in your preferences at https://bugs.kde.org/userprefs.cgi?tab=saved-searches It would also be good if you can do some testing with a clean config/db. Cheers Lydia -- Lydia Pintscher Amarok community manager kde.org - amarok.kde.org - kubuntu.org claimid.com/nightrose ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Fix Inconsistencies with Organize Files Dialog when canceling Dialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100321/#review771 --- I think that It's a bad idea to apply any settings when user press Cancel, so storing Format Presets should stay in onAccept slot. Agree with second statement. And finally this patch has nothing in common with mentioned Bug Report. - Sergey On Jan. 8, 2011, 11:11 a.m., Philipp Schmidt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100321/ > --- > > (Updated Jan. 8, 2011, 11:11 a.m.) > > > Review request for Amarok. > > > Summary > --- > > Fixes two errors: > > First: Presets are being saved explicitely, meaning they should persist even > when the Dialog is aborted/canceled. > Second: The state of the Current Collection Directory is saved regardless of > whether the Dialog was accepted or canceled. IMO it should only be saved like > all other values when it is accepted. > > > This addresses bug 255325. > https://bugs.kde.org/show_bug.cgi?id=255325 > > > Diffs > - > > ChangeLog 3c337d1 > src/dialogs/OrganizeCollectionDialog.cpp b7d7850 > > Diff: http://git.reviewboard.kde.org/r/100321/diff > > > Testing > --- > > > Thanks, > > Philipp > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Review Request: Fix Inconsistencies with Organize Files Dialog when canceling Dialog
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100321/ --- Review request for Amarok. Summary --- Fixes two errors: First: Presets are being saved explicitely, meaning they should persist even when the Dialog is aborted/canceled. Second: The state of the Current Collection Directory is saved regardless of whether the Dialog was accepted or canceled. IMO it should only be saved like all other values when it is accepted. This addresses bug 255325. https://bugs.kde.org/show_bug.cgi?id=255325 Diffs - ChangeLog 3c337d1 src/dialogs/OrganizeCollectionDialog.cpp b7d7850 Diff: http://git.reviewboard.kde.org/r/100321/diff Testing --- Thanks, Philipp ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
Re: Review Request: Reduce code duplication (combo box list)
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100309/#review770 --- Ship it! Looks good to me. At first I thought, this introduces string changes, we cannot do it before 2.4.0 release. But then I realized that you have just removed strings, but not changed any. One nitpick: You should probably change "" to QString(). - Mark On 2011-01-06 20:27:28, Alexander Potashev wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100309/ > --- > > (Updated 2011-01-06 20:27:28) > > > Review request for Amarok. > > > Summary > --- > > Reduce code duplication when filling in items of combo box. > > > Diffs > - > > src/widgets/MetaQueryWidget.cpp 8868106 > > Diff: http://git.reviewboard.kde.org/r/100309/diff > > > Testing > --- > > > Thanks, > > Alexander > > ___ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
