itsme-zeix / pe

0 stars 0 forks source link

Developer Guide lacks implementation details of command parsing/flow of validation logic #10

Open itsme-zeix opened 2 weeks ago

itsme-zeix commented 2 weeks ago

Developers modifying ClinicBuddy may want to modify parts of certain commands, such as validation logic in find with NRIC. However, the DG does not inform them of where the logic for such commands start or are contained, especially for commands that exist outside of AB-3.

The development team could include more regarding such implementations in the Developer Guide to ensure that it remains helpful to developers.

nus-pe-script commented 2 weeks ago

Team's Response

image.png

We agree that we can be more specific in stating where the logic for each command is located. But like the existing AB-3 commands, the validation logic is located in the respective parser classes as shown in the screenshot above.

Items for the Tester to Verify

:question: Issue severity

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

Reason for disagreement: Thank you for responding to the issue promptly.

I would like to respectfully dispute the classification of this issue as low severity and suggest it be reassessed as medium severity.

The knowledge of the locations of the respective parser classes, while helpful, is insufficient for a developer to quickly understand how the parsing of a command is handled. Critical questions, such as:

are likely to arise for developers attempting to modify or extend the application.

Without clear documentation addressing these points, developers are forced to manually trace the flow of logic through the codebase and investigate each function involved in parsing and validation. This process is not only time-consuming but also risks errors and misunderstandings, especially for developers new to the codebase.

Such inconveniences are not rare, as they can arise every time a developer needs to modify or extend command-related logic. This makes the issue more significant than a "very rare situation" or a "minor inconvenience" as specified under low severity. Instead, the issue aligns with the criteria for medium severity, which involves "occasional inconvenience to some users."

Reclassifying this issue as medium severity would more accurately reflect the inconvenience caused to developers and emphasize the need for clearer documentation to ensure smooth and efficient modifications to the codebase.

Thank you for considering this clarification.