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 phone model #62

Merged
merged 6 commits into from Sep 27, 2019
Merged

Add phone model #62

merged 6 commits into from Sep 27, 2019

Conversation

yan-wl
Copy link

@yan-wl yan-wl commented Sep 26, 2019

No description provided.

@yan-wl yan-wl requested a review from a team September 26, 2019 13:32
EugeneTeu
EugeneTeu reviewed Sep 26, 2019
View reviewed changes
Copy link

@EugeneTeu EugeneTeu left a comment •
edited

Choose a reason for hiding this comment

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

Need to confirm Enum format of Cost.java

@EugeneTeu
Copy link

Generally okay, need to confirm the above things before merge

EugeneTeu
EugeneTeu reviewed Sep 27, 2019
View reviewed changes
Copy link

@EugeneTeu EugeneTeu left a comment

Choose a reason for hiding this comment

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

need to Create custom colour class

@EugeneTeu EugeneTeu requested review from EugeneTeu and removed request for EugeneTeu September 27, 2019 16:03
EugeneTeu
EugeneTeu requested changes Sep 27, 2019
View reviewed changes
Copy link

@EugeneTeu EugeneTeu left a comment

Choose a reason for hiding this comment

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

I think the check style alert is being thrown cause of some import rules

EugeneTeu
EugeneTeu approved these changes Sep 27, 2019
View reviewed changes
Copy link

@EugeneTeu EugeneTeu left a comment

Choose a reason for hiding this comment

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

good catch to use nonNull

@EugeneTeu EugeneTeu merged commit c02a88a into AY1920S1-CS2103T-T09-4:master Sep 27, 2019
@nus-se-bot nus-se-bot mentioned this pull request Oct 2, 2019
Open
34 tasks
@EugeneTeu EugeneTeu added this to the v1.2 milestone Oct 9, 2019
@EugeneTeu EugeneTeu added the priority.High Must do label Oct 9, 2019
@nus-se-bot nus-se-bot mentioned this pull request Oct 9, 2019
Open
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiujiaaa qiujiaaa qiujiaaa left review comments

@EugeneTeu EugeneTeu EugeneTeu approved these changes

Assignees

@yan-wl yan-wl

Labels
priority.High Must do
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yan-wl @EugeneTeu @qiujiaaa