-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ladislas/feature/contentkit refactor activity exercise data models #456
ladislas/feature/contentkit refactor activity exercise data models #456
Conversation
ladislas
commented
Nov 13, 2023
- 🚨 (ContentKit): Sonarcloud - Rename "activity" which has the same name as the field
- 🚚 (ContentKit): Reorganize files between Activity and exercise
- ♻️ (ContentKit): Move ExerciseSequence inside Exercise
- ♻️ (ContentKit): Rename ExerciseType to Category + move to Exercise
- 🗃️ (ContentKit): Rename type to category in yaml files
- ♻️ (ContentKit): Move Action to Exercise
- ♻️ (ContentKit): Move Gameplay to Exercise
- ♻️ (ContentKit): Move Payload to Exercise
- ♻️ (ContentKit): Action - Replace robot/iPadMedia by ActionType
- ♻️ (ContentKit): Rename SelectionChoice to TouchSelectionChoice
- ♻️ (ContentKit): Rename SelectionPayload to TouchSelectionPayload
- 🚚 (ContentKit): Rename DragAndDropSelection to DragAndDropIntoZones
- ♻️ (ContentKit): Rename DragAndDropChoice to DragAndDropIntoZonesChoice
- ♻️ (ContentKit): Rename ChoiceDropZone to DropZone
- ♻️ (ContentKit): Rename DragAndDropPayload to DragAndDropIntoZonesPayload
- ♻️ (ContentKit): DragAndDropIntoZones - create enum, move all
- ⚗️ (ContentKit): DragAndDropAssociation - Add basic enum to hold things
- ♻️ (ContentKit): TouchSelection - create enum, move all
28922af
to
80c7aa3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good refactor, a way lighter than before ! Some naming remarks
Modules/ContentKit/Sources/Exercise/Exercise+DragAndDropAssociation.swift
Outdated
Show resolved
Hide resolved
Modules/ContentKit/Sources/Exercise/Exercise+DragAndDropAssociation.swift
Show resolved
Hide resolved
Modules/ContentKit/Sources/Exercise/Exercise+DragAndDropAssociation.swift
Outdated
Show resolved
Hide resolved
|
||
// swiftlint:disable nesting | ||
|
||
public enum DragAndDropIntoZones { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of this naming. It feels weird next to TouchToSelect, TouchToAssociate, DragAndDropToAssociate that end with a verb. Can it be DragAndDropToAllocate or another verb to describe what we do ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
me neither, but I'm out of ideas for this name. It's not the same as the others, but at the same time I'm not sure we'll be able to always keep this this way. When we'll work on sequencing, or something else, names might differ.
I don't want to have just "drag and drop" as it seems it's on top of the others.
I'll keep this for the moment, we can revisit at a later time if needed when something pops up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Modules/ContentKit/Sources/Exercise/Exercise+TouchAssociation.swift
Outdated
Show resolved
Hide resolved
80c7aa3
to
87a63a3
Compare
87a63a3
to
3eda5df
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |