-
Notifications
You must be signed in to change notification settings - Fork 0
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
major refactor to our codebase for long term development #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Just one thing: does it make sense to contain all the functionality like browse/searching and course viewing within a profile directory? I feel like those two don't really fall into the category, but I do like the idea of separating. So maybe they can be in their own dirs?
Maybe I should rename the profile directory... I do like the idea of keeping them together; I feel like these two are workflows/actions that the user would do. |
I don't see a problem and all looks good to me, though I agree that it's worth discussing whether course search should be within profile. I was thinking maybe you'd have the profile be one folder and course search be another? Once this is sorted out I'm ready to approve this to merge. |
Yeah, I feel your (Sam) page (to be renamed Home) would make the most sense under "Profile" and Guanting and I's pages would be more logical under the main directory. But also remember that these are Pages, more general than the Components within them, and much fewer of them to try to group by function. Perhaps it makes sense to not really be generating too many subdirs for the Pages? |
I agree with you that Browse/Search is too general to be in another profile subdirectory, but I think they are components within a user's actions. I don't think they would be pages because our page should only refresh the user's actions (browsing, or course searching) all the while a navbar and other things on the page will remain the same. So, why don't we put Browse/Search within the component folder for now, and for future iterations, if they do become standalone general components, why don't we create folders for themselves? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@baconandchips 's VisualizeCourse pieces will need to be moved to Components later.
And all of our pages will be need to be modularized as much as reasonably possible later.
Looks good with me as well! I agree on the component move; will be doing that when I get off work. I think having Profile might be a bit general for the components as of now; maybe we can start off with having page components in folders for their respective pages under "Components" folder, and as we develop commonly-used components we can perhaps move them to the main "Components" folder, or we develop them directly in the "Components" folder. We can definitely discuss more in the meeting to agree on a standard |
No description provided.