search

WZWren / pe

0 stars 0 forks source link

DG does not properly explain how the Matrix is generated #11

Open WZWren opened 9 months ago

WZWren commented 9 months ago

In the DG, it is expected that the matrix should be well documented, given that the matrix occupies a large part of the UI space. However, it is not included in the UI section of the high-level architecture explanation.

image.png

It is also not mentioned in the Implementation section of the DG.

image.png

On inspection of code, we see that it is included as a TableView and likely GridPane in the MainWindow.java file, which seems to lack its own abstraction without explanation as to why.

image.png

This is a big problem for developers working on this aspect of the program in the future, as the UI sections of the program also tend to lack comments.

nus-pe-bot commented 9 months ago

Team's Response

Thank you for pointing this out! We assign this as NotInScope as we feel that it is much lower in priority than the work already done in v1.4. We acknowledge that this is not ideal but it was a decision we took to leave out the diagram and implementation for matrix for this version. The matrix is one of our proprietary features, and we wanted to not rush with its documentation, and pushed it to later. It would have been too complex to include the tableview and gridpane in this version so we went with a higher level diagram. Hence, we feel it is not in scope under v1.4 but nevertheless, thank you for bringing to our attention.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: It does not make sense to delay the diagram and implementation of Matrix, especially if its one of your proprietary features.

If you were to onboard a new developer onto your in v1.4, they would be unable to check the documentation of this feature and see how their changes to other systems might affect this important feature of your app. I believe this should not be NotInScope but an Accepted, since this isn't a case where the matrix was briefly mentioned but not elaborated on in their DG, but a case where the matrix is not mentioned at all, and the team has also acknowledged this issue.

## :question: Issue severity Team chose [`severity.Medium`] Originally [`severity.High`] - [x] I disagree **Reason for disagreement:** No proper justification for the reduction of severity was given. I stand by this severity as `High` and double-down on this severity, as a complete lack of documentation of a *key-feature* of the app is completely unacceptable for any developer looking to join the team.