search

gbowne1 / RadioLogger

A Radio Logging application build with NodeJS and ExpressJS
GNU General Public License v3.0
6 stars 6 forks source link

Added a profile and follower feature. Also added a follower model #72

Closed Joywin2412 closed 11 months ago

jzunigarce commented 11 months ago

Please follow the project structure

gbowne1 commented 11 months ago

@jzunigarce what was wrong with that?

jzunigarce commented 11 months ago

I'm mentioning it because of what we talked about yesterday, we have several files such as controllers, services where we delegate functionalities, for example, the services are the ones that interact with the models, while the controllers interact with the services. If, for example, we establish the connection data in the controllers, the application would not be as scalable. We currently have a configuration file for connecting to the db so as not to duplicate so much code, also if you want to change any data such as the name of the db you would have to do it in each controller in which it is being implemented

gbowne1 commented 11 months ago

Then I would say the documentation needs to address the structure. I did put some on the project wiki. The structure is a bit of a mess.

The PR was for the profile page implement a follow button that exists on the /profile.

I would not just blindly close a PR till the contributor is aware of the changes that need made. Just do a review on Github and select the radio button that says needs changes.

@Joywin2412 @jzunigarce

Joywin2412 commented 11 months ago

Please follow the project structure

Okay I'll check and review my code and do pull request again

gbowne1 commented 11 months ago

I think the code in the PR was pretty good overall.

The structure needs documented so that newcomer contributors can follow the correct approach.

gbowne1 commented 11 months ago

To be fair I hadn't documented much of this app yet.

Whom ever gets permanently assigned to maintain the backend would have a better time when it gets properly documented.

jzunigarce commented 11 months ago

I accept the PR. Yes, we need work with the documentation

gbowne1 commented 11 months ago

The wiki might be easiest to keep updated. All you have to so do is clone the wiki repo and edit the markdown and then just link the wiki in the README.md

I need to do this for all of my others apps.

We can always do some refactoring on incoming PRs.

I still want to implement e2e tests, unit tests and integration tests. We are using mocha and chai here which I still need to learn.