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

[T10-3] LearnVocabulary #109

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

Conversation

russellry
Copy link

@russellry russellry commented Oct 3, 2018

Travis Build is still failing - even before coding, needs a little more attention.

Code wise:
Changes implemented - dictionary function, add/find, grouping and trivia

Comments:
Dictionary has been fully worked out, just needs to be integrated, will probably need a lot of debugging.
Add/find, grouping and trivia has a few bugs

Also, we are aware of some discrepencies in the naming of our document and will address that by next week! We will also update the author's name as well.

Copy link

@parvathy-sudhir-pillai parvathy-sudhir-pillai left a comment

Choose a reason for hiding this comment

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

Overall:
Good job on meeting all requirements for the v1.1 team component. However, some areas need more work. Hope you can tackle those areas in upcoming features.

Some detailed feedback:

  • ReadMe adoc
  1. Product description still refers to AB-4's functionality (point 2).
  2. Describe what LearnV is in the homepage
  3. Remove top SE EDU nav bar and link to LOs
  • View on GitHub link
    Results in 404. Check that you have correctly followed the steps for auto-publishing of docs

  • Contact Us adoc
    Details have to be updated

  • About Us adoc

  1. Photos aren't visible. Please update ASAP.
  • Developer Guide adoc
  1. Looks like the auto publish is not properly enabled. The page is all cluttered up and not being displayed in the correct manner.
  2. Two groups of Group User stories provided. Use either one, preferably the one grouped by priority
  3. Use case steps are too complex and not atomic. Suggest splitting up into multiple steps as in the comment provided.

. Go to `File` > `Settings...` (Windows/Linux), or `IntelliJ IDEA` > `Preferences...` (macOS)
. Select `Editor` > `Code Style` > `Java`
. Click on the `Imports` tab to set the order
System checks if word exists in the dictionary and displays if the word could be successfully added or not. It also displays the meaning of the word.

Choose a reason for hiding this comment

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

Too many steps in this one line. Ideally it should be split up as the MSS and the exception workflow. Something like:
MSS:

  1. User enters a word to add to the review list.
  2. System adds word to the dictionary.
  3. System prompts user with word successfully added message
  4. System displays newly added word's meaning
    Use case ends

Extensions:
2a. Word already exists in the dictionary
2a1. System prompts user with word already exists message
Use case ends

Copy link
Author

Choose a reason for hiding this comment

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

@russellry
Copy link
Author

Noted:
Ui.png has been updated (❗️ try to do by next milestone)
Milestones created: v1.2 [Please ensure milestones are setup until 'v1.4'] (❗️ try to do by next milestone)
Issues marked as type.Story e.g., (❗️ try to do by next milestone)
Issues allocated to v1.3 e.g., (❗️ try to do by next milestone)
Build passing (❗️ try to do by next milestone)

Individual Progress:
Gave review comments to other PRs (❗️ try to do by next milestone) - What is this exactly? On this PR? Or on other people's PR?

Copy link

@parvathy-sudhir-pillai parvathy-sudhir-pillai left a comment

Choose a reason for hiding this comment

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

Feedback for v1.2

Read Me ascii doc

  • The Introduction section refers to a Section 2, “Quick Start” which is not part of Read Me, but in User guide. Update description
    t10_1

User guide ascii doc

  • Use better descriptions for what each command does. Some of them are not very clear. For e.g. Group words. The description reads group words accordingly. According to what?
    t10_2

Developer guide ascii doc

  • If a feature is not implemented, you can mark it as coming in v2.0
  • User stories are repeated twice. Use the version in the Appendix as they are prioritised.
    t10_3
    t10_4

Overall

  • Good effort. Work on getting the product to a user-testable state by v1.3
  • Catch up on documentation work. Good job adding some design descriptions, but not enough new UML diagrams to give feedback on your UML modeling skills. Try to add more soon.

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