add code documentation and minimal code smells improvement
AbandonedPublic

Authored by pedrohenriques on Aug 19 2019, 4:01 PM.

Details

Reviewers
None
Group Reviewers
KDE Games

Diff Detail

Repository
R392 KBounce
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15307
Build 15325: arc lint + arc unit
pedrohenriques created this revision.Aug 19 2019, 4:01 PM
Restricted Application added a reviewer: KDE Games. · View Herald TranscriptAug 19 2019, 4:01 PM
Restricted Application added a subscriber: kde-games-devel. · View Herald Transcript
pedrohenriques requested review of this revision.Aug 19 2019, 4:01 PM

Thanks in advance for fixing these minor issues. You might want to change the review request title as well.

ball.cpp
187

Same here.

ball.h
100

That should be vX and vY to avoid Doxygen complaining about the correspondence of parameters.

board.cpp
42

@param renderer

board.h
115

@param velocity

122

@param velocity

renderer.cpp
44

This line does not belong here.

Thanks indeed. Just a note for future. Try to keep patches smaller. Separate them in e.g. whitespace changes, documentation changes and code changes. git add -p or git commit -p helps with that.
Reason: If you mix code changes with whitespace changes and then the code change turns out to be problematic, you cannot just revert the change without losing the whitespace fixes.

ball.cpp
111–114

This is valid C++ and some style guides allow leaving out the braces ... however, during my day job I found cases, where the code was altered later and the developer added one line and forgot about adding braces. So I would always use braces on an if statement. ... But this is just my opinion.

gamewidget.cpp
39–48

You kept the spaces inside the braces here. On purpose?

aacid added a subscriber: aacid.Aug 19 2019, 7:51 PM

this is unreviawable, please split it in different parts

board.cpp
47

this comment is useless

board.h
218

this rename is useless

pedrohenriques abandoned this revision.Wed, Sep 11, 6:15 PM
pedrohenriques marked 8 inline comments as done.

Thank you all for the responses, it's my first time trying to contribute with KDE and I focused mainly on doing the phabricator works on my computer and understanding the code that I forgot to made a good revision (It was my fault). I'll send this changes in another revision with organized commits, but for now this revision can be closed.

Ps.: If you have another tips about how to contribute, I'll be glad to learn.