search

jwdavis0200 / pe

0 stars 0 forks source link

Confusing use of `/t` parameter for different commands. #4

Open jwdavis0200 opened 2 years ago

jwdavis0200 commented 2 years ago

For the ap feature, /t is used for issuing a tag to the patient to be entered. Other features such as edit defines /t in this way.

For the aa feature, /t is used adding medical tests to the patient as states in the UG below. image.png

nus-pe-script commented 1 year ago

Team's Response

In real world CLI commands, sharing same prefixes among different commands is a common practice.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Could minimise repetition for parameter list by combining parameter requirements as a summary table

Note from the teaching team: This bug was reported during the Part II (Evaluating Documents) stage of the PE. You may reject this bug if it is not related to the quality of documentation.

Several of the commands have similar parameters such as name, email, address or date or money.

This results in quite a bit of duplication of the parameter list for each and every command, making the user guide quite long to scroll and more of a hassle to scroll (going to the content table at the beginning helps a bit).

The parameter requirements could be concisely written below with a link pointing to it. This reduces the amount of repetition. At the same time, the parameter list for each command could summarise which is optional or mandatory.

[original: nus-cs2103-AY2223S1/pe-interim#5384] [original labels: severity.VeryLow type.DocumentationBug]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

In the real world software, shared prefixes are quite common among different commands in practice. We use shared prefixes using the same characters for different commands, and hence giving a summarised parameter list for each command would make it clearer.

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: My report is completely different from the bug report that was reported by the other tester.

My bug report comes from a user perspective. The other bug report by the other user focuses on the user-friendly aspect of the team's documentation.

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: I agree that in real world software, shared prefixes are not very rare (I would refrain from saying shared prefixes are quite common among good commercial products, you could say that about pressing certain keys to execute different commands in different modes yes in CLI apps but prefixes being common I am not so sure).

But in the case of your app, there are many better ways to implement the prefixes such that new users would not be confused about your user of /t for tags vs medical tests for different commands (which I would think is likely to cause an inconvenience to the users). for example, you could use mt/ for your medical tests or if you would like to stick to a single character prefix maybe m/ instead.

:question: Issue type

Team chose [type.DocumentationBug] Originally [type.FeatureFlaw]

Reason for disagreement: I am not coming from a User Guide perspective. This comes from a new user perspective who may be confused with the way you use /t prefix for both tags and medical tests for different commands etc.

:question: Issue severity

Team chose [severity.VeryLow] Originally [severity.Low]

Reason for disagreement: This is not a cosmetic issue. Please refer to the definitions below stated by the module website.

image.png

My bug report clearly refers to a user using the app.