Review Request: Partial Rewrite of ID3v2TagHelper.cpp

[Ralf Engels]

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


Since we just released 2.6 I go though all the review requests.
I noticed that you done some re-formating and added "title", "genre", "tracknumber" and so on.
However those tags are already handled via the taglib title() and genre() function. They don't need special handling in our handlers.
So it seems that the "real" content of the patch is quite slim. It would be just some small format changes.

Also you didn't answer my question: Do the auto tests still run?

Please indicate what you want to do with the request.

- Ralf Engels


On Aug. 3, 2011, 9:20 p.m., Stefan Derkits wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102055/
> -----------------------------------------------------------
> 
> (Updated Aug. 3, 2011, 9:20 p.m.)
> 
> 
> Review request for Amarok and Ralf Engels.
> 
> 
> Description
> -------
> 
> Rewrite if else else if Constructs to switch statements
> 
> Add common Frame Names to m_fieldMap
> 
> 
> Diffs
> -----
> 
>   shared/tag_helpers/ID3v2TagHelper.cpp 27e0cf0a9f6dc0bfbbd54cab65cb65dd6bef8a33 
>   shared/tag_helpers/TagHelper.h f8e7fd9fdad0ac3abd6efd34ad5e00c973233a6d 
>   shared/tag_helpers/TagHelper.cpp 96f763e0839b59194484c1aa94cd266d88ad5add 
>   shared/tag_helpers/VorbisCommentTagHelper.cpp 1222f9fc9e0b3f04499d9542e29e58b12d8c5f1b 
> 
> Diff: http://git.reviewboard.kde.org/r/102055/diff/
> 
> 
> Testing
> -------
> 
> compared output of collectionscanner before & after patch -> fixed some, but still some slight problems with TrackNo
> 
> 
> Thanks,
> 
> Stefan Derkits
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120816/0c052cac/attachment.html>