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
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? | |
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.