Skip to content
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

[CS2113-F11-1] FITNUS #27

Open
wants to merge 325 commits into
base: master
Choose a base branch
from

Conversation

MrOPPA1
Copy link

@MrOPPA1 MrOPPA1 commented Oct 6, 2023

No description provided.

@MrOPPA1 MrOPPA1 changed the title [CS2113-F11-1] InsureList [CS2113-F11-1] NUSFIT Oct 6, 2023
@MrOPPA1 MrOPPA1 changed the title [CS2113-F11-1] NUSFIT [CS2113-F11-1] FITNUS Oct 6, 2023
Brian030601 added a commit to Brian030601/tp that referenced this pull request Oct 19, 2023
MrOPPA1 and others added 26 commits October 20, 2023 02:20
Enable assertions in build.gradle
Add comments to ExerciseLog and associated classes
Fix errors
Allow users to update exercises within the exercise log
Copy link

@itayrefaely itayrefaely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. First, it feels like there are text blocks in the DG which are maybe too long, not organized in bullet points, and are not very technical. Is it possible that this makes it more difficult to understand?
Screenshot 2023-11-01 at 12 18 46
  1. The DG is missing a few key components, such as target user profile, value proposition, and more.
Screenshot 2023-11-01 at 12 19 58 Screenshot 2023-11-01 at 12 21 15
  1. The user stories for versions v1.0 and v2.0 are not related to what appears in the rest of the DG.

  2. It seems that the approach you took to make a command superclass is very well explained in the DG, including the logic/reasoning, and also it's implementation structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants