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

Course-related commands on new model/storage impl. Added tests. Updated docs. #99

Merged
merged 4 commits into from
Oct 17, 2018
Merged

Course-related commands on new model/storage impl. Added tests. Updated docs. #99

merged 4 commits into from
Oct 17, 2018

Conversation

m-aslam-mj2
Copy link

  • moved course-related code to new model/storage implementation.
  • added basic tests for course add command.
  • updated docs.
  • refactored person-related XML files to adhere to convention of other XML classes.

@m-aslam-mj2
Moved course management related commands to new model/storage impleme…
a1b809c 
…ntation. Added several tests. Updated docs.

Signed-off-by: m-aslam-mj2 <e0176811@u.nus.edu>
@m-aslam-mj2
Included files affected by refactoring of Person classes.
5caa692 
Signed-off-by: m-aslam-mj2 <e0176811@u.nus.edu>
@m-aslam-mj2 m-aslam-mj2 added Changes in Documentation This PR contains changes to documentation Changes in Shared Code This PR contains changes to shared code Changes in Code This PR contains changes to code Changes in Testing Code This PR contains changes to testing code labels Oct 17, 2018
@m-aslam-mj2 m-aslam-mj2 added this to the v1.2 milestone Oct 17, 2018
@m-aslam-mj2 m-aslam-mj2 requested a review from a team as a code owner October 17, 2018 05:49
@coveralls
Copy link

coveralls commented Oct 17, 2018 •
edited

Coverage Status

Coverage increased (+0.4%) to 73.364% when pulling b383a7f on m-aslam-mj2:staging into aec8e41 on CS2113-AY1819S1-T16-1:master.

harriuscai
harriuscai previously approved these changes Oct 17, 2018
View reviewed changes
Copy link
Collaborator

@harriuscai harriuscai left a comment

Choose a reason for hiding this comment

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

Props on adding some basic tests 👍

src/main/java/seedu/address/storage/serializable/XmlSerializableCourseList.java
}



Copy link
Collaborator

Choose a reason for hiding this comment

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

so much whitespace :O

@harriuscai harriuscai added the Approved with Minor Comments This PR contains comments by reviewer for potential improvements. label Oct 17, 2018
@harriuscai harriuscai merged commit 731caf8 into CS2113-AY1819S1-T16-1:master Oct 17, 2018
@m-aslam-mj2 m-aslam-mj2 deleted the staging branch October 17, 2018 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harriuscai harriuscai harriuscai approved these changes

Assignees
No one assigned
Labels
Approved with Minor Comments This PR contains comments by reviewer for potential improvements. Changes in Code This PR contains changes to code Changes in Documentation This PR contains changes to documentation Changes in Shared Code This PR contains changes to shared code Changes in Testing Code This PR contains changes to testing code
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@m-aslam-mj2 @coveralls @harriuscai