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 Organization class #34

Merged
merged 22 commits into from
Oct 17, 2023
Merged

Add Organization class #34

merged 22 commits into from
Oct 17, 2023

Conversation

tanshiyu1999
Copy link

No description provided.

@tanshiyu1999 tanshiyu1999 added the priority.High Must do tasks (CS2103T label) label Oct 13, 2023
@tanshiyu1999 tanshiyu1999 added this to the v1.2 milestone Oct 13, 2023
@McNaBry McNaBry changed the title Add status class Add Organization class Oct 13, 2023
tanshiyu1999 and others added 2 commits October 14, 2023 14:07
@McNaBry McNaBry marked this pull request as draft October 14, 2023 14:56
@wxwern wxwern added the type.Enhancement Enhancement to an existing feature or user story (CS2103T label) label Oct 15, 2023
wxwern
wxwern reviewed Oct 15, 2023
View reviewed changes
Copy link

@wxwern wxwern left a comment

Choose a reason for hiding this comment

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

Code structure looks alright. I've included some miscellaneous feedback. Do remember to remove any leftover debugging or commented out code before finalizing.

src/main/java/seedu/address/model/person/Organization.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/model/person/Position.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/AddCommandParser.java Outdated Show resolved Hide resolved
@CJ-Lee01 CJ-Lee01 linked an issue Oct 16, 2023 that may be closed by this pull request
As a user, I can add contacts #12
Closed
11 tasks
@McNaBry McNaBry marked this pull request as ready for review October 17, 2023 03:14
wxwern
wxwern approved these changes Oct 17, 2023
View reviewed changes
Copy link

@wxwern wxwern left a comment

Choose a reason for hiding this comment

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

Should be good to go now

@wxwern wxwern requested a review from wamps-jp October 17, 2023 06:52
McNaBry
McNaBry approved these changes Oct 17, 2023
View reviewed changes
Copy link

@McNaBry McNaBry left a comment

Choose a reason for hiding this comment

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

Overall, looks good to me.

@wxwern
Copy link

wxwern commented Oct 17, 2023 •
edited
Loading

Seems like the old "first time use example data" will no longer correctly load - that can be fixed in a future PR since it isn't exactly critical.

@tanshiyu1999 tanshiyu1999 merged commit 26c02bf into AY2324S1-CS2103T-W08-3:master Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wxwern wxwern wxwern approved these changes

@McNaBry McNaBry McNaBry approved these changes

@wamps-jp wamps-jp Awaiting requested review from wamps-jp

Assignees
No one assigned
Labels
priority.High Must do tasks (CS2103T label) type.Enhancement Enhancement to an existing feature or user story (CS2103T label)
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

As a user, I can add contacts
3 participants
@tanshiyu1999 @wxwern @McNaBry