Review Request 121250: SoK Implement Cuesheet Support

[Vedant Agarwala]

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



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

    Correct the indentation



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

    From where have you gotten this code?
    
    paste a link as a comment in begining of any code sopied from somewhere else.



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

    Replace the numbers with #define constants. In this line and the lines before it



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

    again. link to original code



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

    suddenly you used a tab?


You said you copied code from /src/core-impl/......
Do not just copy. Delete the code from there. I didn't see any deleted lines. Did you find usages of the class in other places? You could have moved that entire class here if it was redundant.

You can move code. But do not keep 2 copies of the same code in the library.

I had explained this to you in email in the mailing list.

Nitul, even when I review I will miss some things. Do not just copy code. I am feeling very bad repeating myself. You should feel the same duplicating code.

And finally, how does it feel to see your code compile? ;-)

- Vedant Agarwala


On Dec. 7, 2014, 6:52 p.m., Nitul Datt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121250/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2014, 6:52 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
> -----
> 
>   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/20141207/32491a1d/attachment-0001.html>