added multipledataset to chronos
Details
Details
- Reviewers
jjazeix echarruau timotheegiet - Group Reviewers
GCompris: Improvements - Maniphest Tasks
- T12715: Add multiple datasets to chronos
Diff Detail
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
Comment Actions
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. | |
Comment Actions
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?
Comment Actions
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.
| src/activities/chronos/resource/2/Data.qml | ||
|---|---|---|
| 32 | if the keys are indexes, it is better to use an array instead | |
Comment Actions
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 | |
Comment Actions
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?
Comment Actions
Thank you, committed in a55e97147139c8a1c3f7b88ce0bc7aacdfaa7e35
| src/activities/babymatch/Babymatch.qml | ||
|---|---|---|
| 104 ↗ | (On Diff #78496) | extra space |