Skip to content
This repository was archived by the owner on Apr 8, 2026. It is now read-only.

Pull request for code review#41

Open
TeoDoughty wants to merge 147 commits into
Facadefrom
master
Open

Pull request for code review#41
TeoDoughty wants to merge 147 commits into
Facadefrom
master

Conversation

@TeoDoughty

Copy link
Copy Markdown
Collaborator

No description provided.

@eitan12345om

Copy link
Copy Markdown
Owner

Matt,

You made some awesome contributions to this project. I really appreciate your work on the LoginActivity Page; this was an integral part of our application. We could not have integrated a login system without your valuable contributions.
I would like to suggest that you make smaller commits so that each change can be reviewed individually. Right now, the commit I reviewed is large and contains many lines of code doing different things. Having a smaller commit would make it easier for me to determine what you are trying to do with each commit.
Additionally, it would be helpful if you had separate commits for different files if they are doing different things and/or are part of different additions to features.
I would also like to suggest that you document your contributions with comments conforming to our coding style. This will make it easier for you and other developers to pick up from where you left off.
I think your functions are short and sweet and have a modular mouthfeel.
Finally, your overall contributions were timely and important.

Keep up the good work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants