enumeration memory games multiple_dataset
ClosedPublic

Authored by dekumar on Jun 2 2020, 5:25 AM.

Diff Detail

Repository
R2 GCompris
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dekumar created this revision.Jun 2 2020, 5:25 AM
Restricted Application added a project: KDE Edu. · View Herald TranscriptJun 2 2020, 5:25 AM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
dekumar requested review of this revision.Jun 2 2020, 5:25 AM
dekumar retitled this revision from Added data1 to enumeration memory games multiple_dataset.Jun 2 2020, 5:26 AM
dekumar edited the summary of this revision. (Show Details)
jjazeix added inline comments.Jun 2 2020, 7:40 AM
src/activities/memory-enumerate/resource/1/Data.qml
29

string

src/activities/memory/MemoryCommon.qml
70 ↗(On Diff #83193)

is there a reason to check a dataset against 0?

dekumar updated this revision to Diff 83198.Jun 2 2020, 12:31 PM
  • Updated datatype
dekumar updated this revision to Diff 83199.Jun 2 2020, 12:36 PM
  • Updated datatype
dekumar marked an inline comment as done.Jun 2 2020, 12:40 PM
dekumar added inline comments.
src/activities/memory/MemoryCommon.qml
70 ↗(On Diff #83193)

@jjazeix For all those activity which are not having multiple datasets needs to checked before loading the datasets. I have added this condition to check for all those memory activities which do not have any multiple datasets.

@jjazeix Are the code changes and the implementation fine? If yes can I push this to my gsoc branch and work on the datasets of addition memory activity?

This revision was not accepted when it landed; it landed in state Needs Review.Jun 5 2020, 5:04 AM
Closed by commit R2:af7308333034: Added data1 (authored by dekumar). · Explain Why
This revision was automatically updated to reflect the committed changes.
dekumar reopened this revision.Jun 5 2020, 5:11 AM
dekumar updated this revision to Diff 83218.Jun 5 2020, 5:43 AM
  • Updated datatype
  • Updated multiple dataset content

@jjazeix I have updated the multiple datasets content. In order to have minimum 2 pairs for the multiple data1 and data2 to satisfy the condition to check minimum data pairs I have repeated the numbers. Also I have removed the number 0 until number 4 as per the suggestions by @echarruau

This revision was not accepted when it landed; it landed in state Needs Review.Jun 5 2020, 5:50 AM
This revision was automatically updated to reflect the committed changes.
dekumar reopened this revision.Jun 6 2020, 7:24 AM
dekumar updated this revision to Diff 83228.Jun 6 2020, 7:24 AM
  • Updated datatype
  • Updated multiple dataset content
  • improved code, updated few dataset contents
dekumar updated this revision to Diff 83229.Jun 6 2020, 7:26 AM
  • Typo fixed
This revision was not accepted when it landed; it landed in state Needs Review.Jun 6 2020, 7:27 AM
This revision was automatically updated to reflect the committed changes.
jjazeix added inline comments.Jun 7 2020, 8:32 PM
src/activities/memory/memory.js
73

extra space added

dekumar added inline comments.Jun 8 2020, 4:04 PM
src/activities/memory/memory.js
73

I have fixed this on my branch in a commit.

Changes are ok.
I would still need to see the process test table where you will document how you do the tests to be sure that you are not breaking any of the other memory activity.

src/activities/memory/memory.js
73

I still see it, we need to find a way to see your corrections

Code looks good to me, I will need to test it more later.
Thank you

Changes are ok.
I would still need to see the process test table where you will document how you do the tests to be sure that you are not breaking any of the other memory activity.

I have fixed that in this commit https://phabricator.kde.org/R2:8c7cdc63d0fff90f1b22fc3afa66ff7625bc03f5
Not getting why it's not updated over here.

Hi,
Maybe because you use a diff that you commit? No idea on my side but
could you ask Timothée or Johnny to kbao why? What is sure is that I
don't see it.

Le mar. 9 juin 2020 à 11:45, Deepak Kumar
<noreply@phabricator.kde.org> a écrit :

dekumar added a comment. View Revision

In D29840#674920, @echarruau wrote:

Changes are ok.
I would still need to see the process test table where you will document how you do the tests to be sure that you are not breaking any of the other memory activity.

I have fixed that in this commit https://phabricator.kde.org/R2:8c7cdc63d0fff90f1b22fc3afa66ff7625bc03f5
Not getting why it's not updated over here.

REPOSITORY
R2 GCompris

REVISION DETAIL
https://phabricator.kde.org/D29840

To: dekumar, GCompris: Improvements, jjazeix, timotheegiet, AkshayCHD, amankumargupta, echarruau
Cc: kde-edu, narvaez, apol