Review Request: Add Category and NewGame classes

Arjen Hiemstra djfreestyler at gmail.com
Sun Jul 3 15:26:36 CEST 2011



> On July 2, 2011, 8:18 p.m., Laszlo Papp wrote:
> > player/lib/category.cpp, line 36
> > <http://git.reviewboard.kde.org/r/101816/diff/2/?file=25869#file25869line36>
> >
> >     Separate lines

Not really necessary. In fact, I prefer to have them on a single line for less than about 5 initialisers, since that reads far more like a sentence.


> On July 2, 2011, 8:18 p.m., Laszlo Papp wrote:
> > player/lib/category.cpp, line 66
> > <http://git.reviewboard.kde.org/r/101816/diff/2/?file=25869#file25869line66>
> >
> >     Separate lines for the readability as well

See #36


> On July 2, 2011, 8:18 p.m., Laszlo Papp wrote:
> > player/lib/remotegame.cpp, line 93
> > <http://git.reviewboard.kde.org/r/101816/diff/2/?file=25871#file25871line93>
> >
> >     Whatever the term is after call, match them :)

Not really. The signal and slot names can be perfectly non-aligned, assuming it makes sense for the public API. In this case, it makes far more sense to have a singal "finished" and a slot "upload" since they are both public API.


> On July 2, 2011, 8:18 p.m., Laszlo Papp wrote:
> > player/lib/remotegame.h, line 65
> > <http://git.reviewboard.kde.org/r/101816/diff/2/?file=25870#file25870line65>
> >
> >     I would call it "startFetchExistingGame". -ing twice sounds a bit weird =)

Nope, that is correct English. "startFetchExistingGame" sounds far weirder because it is abbreviated English. What you might want to use would be "fecthExistingGame", but that depends on whether it is important to know it is an activity that is started and will end somewhere in the future or whether that can simply be ignored.


- Arjen


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


On July 2, 2011, 2:29 p.m., Shantanu Tushar Jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101816/
> -----------------------------------------------------------
> 
> (Updated July 2, 2011, 2:29 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Category's job is to fetch list of categories from the server.
> NewGame takes a game name and a category, and then creates a new game on the server, returning the ID of newly created game.
> 
> 
> Diffs
> -----
> 
>   player/lib/CMakeLists.txt 53c0f0a 
>   player/lib/category.h PRE-CREATION 
>   player/lib/category.cpp PRE-CREATION 
>   player/lib/remotegame.h PRE-CREATION 
>   player/lib/remotegame.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101816/diff
> 
> 
> Testing
> -------
> 
> Both classes function as expected when used.
> 
> 
> Thanks,
> 
> Shantanu Tushar
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/gluon/attachments/20110703/8407dfa0/attachment-0001.htm 


More information about the Gluon mailing list