Multiple dataset to chronos
ClosedPublic

Authored by shubhammishra on Feb 28 2020, 8:11 PM.

Details

Summary

added multipledataset to chronos

Diff Detail

Repository
R2 GCompris
Branch
chronos_multipl_dataset
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24235
Build 24253: arc lint + arc unit
shubhammishra created this revision.Feb 28 2020, 8:11 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptFeb 28 2020, 8:11 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
shubhammishra requested review of this revision.Feb 28 2020, 8:11 PM
  • removed space
  • updated dataset difficulty

Please add reviewers too

src/activities/babymatch/Babymatch.qml
35

A generic property should be used instead of having a specific one for each activity.
Babymatch is derived in a lot of activities and if every time we add a dataset for one activity, we need to both add a activityName + find in the code the specific place to add this if, it won't be maintainable.
Having a boolean "useMultipleDataset" would be better.

  • added useMultipleDataset property
shubhammishra added a comment.EditedFeb 29 2020, 9:53 AM

Hi, I have only added the links in data files instead of copy pasting the content of board's files else it will make data files too large and messy but I think this way is quite informal. What do you say?

Hi, I have only added the links in data files instead of copy pasting the content of board's files else it will make data files too large and messy but I think this way is quite informal. What do you say?

Hi, the question to be able to answer is: later, when people can edit dataset (via an editor), will it be easy to generate the file(s)? Here it means we will have to generate at least two files + the complexity of the board.

jjazeix added inline comments.Mar 15 2020, 8:03 PM
src/activities/chronos/resource/2/Data.qml
32

if the keys are indexes, it is better to use an array instead

  • converted dataset objects to arrays

Except those small changes, looks good to me.
@echarruau, @timotheegiet any comment to add?

src/activities/babymatch/Babymatch.qml
94

use parenthesis to separate the different values

src/activities/babymatch/babymatch.js
78

you can directly set items.dataset.source here and below

83

indentation

  • removed extra varible, fixed identation, added paranthesis.
shubhammishra marked 3 inline comments as done.Mar 25 2020, 8:05 PM

My sources are in WIP state now so I can not really test this patch now.
But if it follows what I proposed on the dataset description, which as far as I can see it does, it should be good for me :)

Ah, just one thing: I see the diff includes an "autosave" file (board12_0.qml.autosave), I guess that was a mistake?

  • removed extra file
  • removed extra file
jjazeix accepted this revision.Mar 29 2020, 11:20 AM
src/activities/babymatch/Babymatch.qml
104 ↗(On Diff #78496)

extra space

This revision is now accepted and ready to land.Mar 29 2020, 11:20 AM
jjazeix closed this revision.Mar 29 2020, 11:20 AM