Closed bwgoulet closed 1 month ago
Wow, looks like GitHub does not allow neat comments to be added to files not currently in a PR, so I will manually start writing here:
[x] Add attributions, as well as docstring descriptions, to the model files https://github.com/unc-csxl/csxl.unc.edu/blob/6dc26524cbfe8138c100b3e99fd356bad3bf9cc9/frontend/src/app/ta-application/application.model.ts#L1-L2 https://github.com/unc-csxl/csxl.unc.edu/blob/6dc26524cbfe8138c100b3e99fd356bad3bf9cc9/frontend/src/app/ta-application/rx-applications.ts#L1-L2 See the other files such as organization models for an example of his comment attribution!
Especially, clarify what this is.
[x] Home component renaming and attributions https://github.com/unc-csxl/csxl.unc.edu/blob/6dc26524cbfe8138c100b3e99fd356bad3bf9cc9/frontend/src/app/ta-application/home/application-home.component.ts#L5-L9
This component should not be named home - not sure if I mentioned it in the previous CR, but it would be good to rename this to confirmation dialog to prevent confusion.
Also, add the attribution!
[x] Document the TA Application Component
Since we were on a time crunch, I let this slide - but please add attributions to this file and also docstrings for the fields! In the future, we will need to rewrite this file to lessen the number of fields.
Specifically, add documentation for this function above.
Document this, especially what Omit<>
is and why it is being used here. This has not been used in our codebase before.
Document the rest of the functions in this file!
[x] UTA Notice:
What is this?
[x] The Service
I remember commenting about this in the CR, what are these being used for? We can just inject the academics service instead of using this, and I recommend that this change is made.
Also, add documentation for all functions here.
[ ] Consider Renaming ta-application
to application
To keep with the consistency of the backend (which uses application
for this feature), and allow for future expansion, it might be a good idea to rename the ta-application
folder in the frontend to either application
or applications
.
@bwgoulet, @alshayefaziz, @KrisJordan
@bwgoulet / @alshayefaziz how is the progress here?
@alshayefaziz / @bwgoulet we are closing this PR.
This PR adds documentation to accompany the new TA Application feature, outlining the current implementation and future concerns.