Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Module and ModuleList #61

Merged
merged 3 commits into from
Oct 3, 2020
Merged

Add Module and ModuleList #61

merged 3 commits into from
Oct 3, 2020

Conversation

erinmayg
Copy link
Collaborator

@erinmayg erinmayg commented Oct 2, 2020 •
edited

Fixes #56

@erinmayg erinmayg added this to the v1.2 milestone Oct 2, 2020
@erinmayg erinmayg linked an issue Oct 2, 2020 that may be closed by this pull request
Create Module and ModuleList class #56
Closed
florenciamartina
florenciamartina reviewed Oct 2, 2020
View reviewed changes
Copy link
Collaborator

@florenciamartina florenciamartina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think you are doing great. However, there are some minor changes to be made. Great job!

src/main/java/seedu/address/model/module/Module.java Show resolved Hide resolved
src/main/java/seedu/address/model/module/Name.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/module/Code.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/module/exceptions/DuplicateModuleException.java Outdated Show resolved Hide resolved
justintzuriel
justintzuriel requested changes Oct 2, 2020
View reviewed changes
Copy link

@justintzuriel justintzuriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request for variable name change to follow coding standard (and similar variables to those commented), typo fix

src/main/java/seedu/address/model/module/exceptions/DuplicateModuleException.java Outdated Show resolved Hide resolved
src/test/java/seedu/address/model/module/ModuleTest.java Outdated Show resolved Hide resolved
src/test/java/seedu/address/model/module/ModuleTest.java Outdated Show resolved Hide resolved
src/test/java/seedu/address/model/module/ModuleTest.java Outdated Show resolved Hide resolved
justintzuriel
justintzuriel approved these changes Oct 3, 2020
View reviewed changes
Copy link

@justintzuriel justintzuriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@justintzuriel justintzuriel merged commit 0f7e4fb into AY2021S1-CS2103-T14-1:master Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@florenciamartina florenciamartina florenciamartina left review comments

@justintzuriel justintzuriel justintzuriel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

Create Module and ModuleList class
3 participants
@erinmayg @justintzuriel @florenciamartina