Refactoring MoneyCore to not duplicate the answerArea and storeArea
ClosedPublic
Actions

Authored by smitpatil on Dec 7 2018, 10:43 AM.

Details

Reviewers

jjazeix

Group Reviewers

GCompris: Improvements

Maniphest Tasks

Lint

Lint Skipped

Unit

Unit Tests Skipped

Restricted Application added a subscriber: kde-edu. · View Herald Transcript

smitpatil requested review of this revision.Dec 7 2018, 10:43 AM

The new file that you created MoneyArea.qml isn't included in the diff. Can you do the following:
git add -A
git diff --staged > patch.diff

and after doing this your new file should be included in the diff too. After doing this, can you update the diff?

To revert the changes and bring your repo to original state, do the following:
git reset HEAD
git reset --hard

src/activities/money/MoneyArea.qml
2	It misses the header containing the copyright. Note that for GCi we are not supposed to ask for a "firstName lastName <mail>" as you are underage and we need one in the header (this is why I commit with my name and add a comment in the commit message to thank you). If it is ok with you, you can use your "real name <email>" (or something like "Tux Gci2018 <no-email@no-email.com>") else you can put mine with a comment like "Refactor by Tux, GCi student 2018".
18	if it is an int, use a int

Really nice patch! Good work :)
Once the diff is updated according to comments, it'll be good I guess.

src/activities/money/MoneyArea.qml
2	License header for the file is missing above :) You can copy and paste that from the other files and replace the appropriate fields with your details.
18	Can you replace "var index" with "int index"?
19	Remove trailing whitespaces here.
27	Remove trailing whitespaces here.
35	Remove trailing whitespaces here.
42	Remove trailing whitespaces here.
45	needs space before }
50	Remove trailing whitespaces here.
52	Remove trailing whitespaces here.
56	Empty line not needed here.
58	Remove trailing whitespaces here.

Oops, seems Johnny and I made comments at the same time :p

smitpatil marked an inline comment as done.

smitpatil marked 12 inline comments as done.

This revision is now accepted and ready to land.Dec 8 2018, 10:04 AM

			Path	Packages
A	M		src/activities/money/MoneyArea.qml (89 lines)
M			src/activities/money/MoneyCore.qml (131 lines)