Review Request 121250: SoK Implement Cuesheet Support

Vedant Agarwala vedant.kota at gmail.com
Tue Dec 2 09:20:25 UTC 2014


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



shared/collectionscanner/CueSheet.h
<https://git.reviewboard.kde.org/r/121250/#comment49741>

    do not use relative headers.
    
    use #include "src/core/meta/forward_declarations.h"
    
    you can see in the CMakeLists.txt which directories are searched when headers are to be included by the build system



shared/collectionscanner/CueSheet.h
<https://git.reviewboard.kde.org/r/121250/#comment49742>

    You have a lot of whitespace errors. Which editor do you use?
    
    I Kate you can set the settings to remove trailing whitespaces on save. I think KDevelop would have something similar.



shared/collectionscanner/CueSheet.cpp
<https://git.reviewboard.kde.org/r/121250/#comment49744>

    Classes should be Capitalized. If you ever change this file be sure to fix this.
    
    Do not change if the file is not directly related to your SoK project



src/core-impl/meta/cue/CueFileSupport.h
<https://git.reviewboard.kde.org/r/121250/#comment49743>

    this change is redundant, and more importantly, breaks the coding guidelines.


- Vedant Agarwala


On Dec. 1, 2014, 8:50 p.m., Nitul Datt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121250/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2014, 8:50 p.m.)
> 
> 
> Review request for Amarok and Vedant Agarwala.
> 
> 
> Repository: amarok
> 
> 
> Description
> -------
> 
> This is an initial implementation for a CueSheet class in the CollectionScanner. There are some errors though, which have yet to be sorted out.
> 
> 
> Diffs
> -----
> 
>   src/core-impl/meta/cue/CueFileSupport.h e8699cc 
>   shared/CMakeLists.txt 31ca0f4 
>   shared/collectionscanner/CueSheet.h PRE-CREATION 
>   shared/collectionscanner/CueSheet.cpp PRE-CREATION 
>   shared/collectionscanner/Directory.h faacce4 
>   shared/collectionscanner/Directory.cpp a65884b 
> 
> Diff: https://git.reviewboard.kde.org/r/121250/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Nitul Datt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20141202/dc03aa9c/attachment-0001.html>


More information about the Amarok-devel mailing list