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

Update UG and DG #183

Conversation

AriellaCallista
Copy link
Collaborator

@AriellaCallista AriellaCallista commented Nov 12, 2023

Changes to UG:

  • Reformatted search and create

Changes to DG:

  • Added "Planned Enhancements" section
  • Updated model and storage class diagrams

Copy link

codecov bot commented Nov 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@ketweeen ketweeen added this to the v1.4 milestone Nov 12, 2023
Copy link
Collaborator

@ketweeen ketweeen left a comment

Choose a reason for hiding this comment

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

Please fix formatting issues


Format: `search t/TAGNAME...`
Prefix | Constraints
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider adding type (mandatory/optional) column? Can consider doing a merged column filled with mandatory*

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Regarding this part, technically all fields are optional, but they need at least 1 of the "optional" fields. But I wasn't sure how to word this without making it even more confusing (as flagged by Jun Hiang during our UG consult) so I just got rid of the mandatory/optional column. Any suggestions on how to solve this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I simply added Mandatory* and added a clarifying note below it for delete. You can refer to image below.

image

e.g. `search n/Alex n/Adam st/rejected` is not allowed.
<box type="tip" seamless>
Tip:
* You can combine multiple search categories in a single `search` command.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Incorrect formatting

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noted!

Examples of successful command execution:
1. `search n/alex bernice`
<br>
![search-success-1](images/search-success-1.png)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why but picture doesn't show up

What does it mean to do an `OR` search within a single category and an `AND` search across multiple categories?
It's best to explain this by breaking down an example `search` command!
`search n/alex bernice st/interviewed t/intern` will output applicants whose:
* names contain either Alex `OR` Bernice
Copy link
Collaborator

Choose a reason for hiding this comment

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

Incorrect formatting

Copy link
Collaborator

@ketweeen ketweeen left a comment

Choose a reason for hiding this comment

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

LGTM

@AriellaCallista AriellaCallista merged commit 2f35b6b into AY2324S1-CS2103T-W09-4:master Nov 12, 2023
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.

2 participants