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

Rename seedu.address.model classes #39

Merged
merged 7 commits into from
Oct 1, 2020
Merged

Rename seedu.address.model classes #39

merged 7 commits into from
Oct 1, 2020

Conversation

yongping827
Copy link

Partially address #38.

The following classes, including all related fields and methods, are renamed as:

  • Person → Transaction
  • Name → Name
  • Phone → Amount
  • Tags → Category
  • Email → Date

@yongping827 yongping827 added priority.high 🥇 Overdue changes; fixing this is critical pr.ready-for-review 📝 type.task 📋 Miscellaneous tasks that aren't feature or documentation-related labels Sep 27, 2020
@yongping827 yongping827 added this to the v1.2 milestone Sep 27, 2020
ianyong
ianyong requested changes Sep 27, 2020
View reviewed changes
Copy link
Member

@ianyong ianyong left a comment •
edited

Choose a reason for hiding this comment

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

Update the package names too to ay2021s1-cs2103-w16-3.finesse as discussed.

Edit: My bad, hyphens are illegal characters. How about ay2021s1_cs2103_w16_3.finesse as recommended here?

Edit 2: Decided to rename packages in a separate PR so that we are able to view the diff for actual changes.

wltan
wltan requested changes Sep 27, 2020
View reviewed changes
LICENSE Outdated Show resolved Hide resolved
docs/AboutUs.md Outdated Show resolved Hide resolved
ianyong
ianyong requested changes Sep 27, 2020
View reviewed changes
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/ParserUtil.java Outdated Show resolved Hide resolved
@wltan
Copy link
Member

wltan commented Sep 27, 2020 •
edited

#39 (review)

Update the package names too to ay2021s1-cs2103-w16-3.finesse as discussed.

Edit: My bad, hyphens are illegal characters. How about ay2021s1_cs2103_w16_3.finesse as recommended here?

Recommend doing this in a separate PR. Refactoring the package would generate a large diff and make it difficult to review the model renaming.

@wltan wltan mentioned this pull request Sep 27, 2020
Closed
zhaojj2209
zhaojj2209 requested changes Sep 30, 2020
View reviewed changes
Copy link

@zhaojj2209 zhaojj2209 left a comment

Choose a reason for hiding this comment

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

The UI currently encounters NullPointerExceptions when the application starts up; to resolve, the following files need to be refactored.

PersonListCard.fxml: Rename to TransactionListCard.fxml
PersonListPanel.fxml: Rename to TransactionListPanel.fxml, with a change in line 7
MainWindow.fxml: Change in line 53

src/main/resources/view/MainWindow.fxml Show resolved Hide resolved
zhaojj2209
zhaojj2209 previously approved these changes Oct 1, 2020
View reviewed changes
Copy link

@zhaojj2209 zhaojj2209 left a comment

Choose a reason for hiding this comment

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

LGTM!

wltan
wltan requested changes Oct 1, 2020
View reviewed changes
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
ianyong
ianyong requested changes Oct 1, 2020
View reviewed changes
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/DeveloperGuide.md Outdated Show resolved Hide resolved
docs/diagrams/tracing/LogicSequenceDiagram.puml Outdated Show resolved Hide resolved
docs/tutorials/AddRemark.md Outdated Show resolved Hide resolved
docs/tutorials/TracingCode.md Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/EditCommandParser.java Outdated Show resolved Hide resolved
ianyong
ianyong approved these changes Oct 1, 2020
View reviewed changes
Copy link
Member

@ianyong ianyong left a comment

Choose a reason for hiding this comment

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

LGTM!

wltan added a commit to AY2021S1-CS2103T-W16-3/reports that referenced this pull request Oct 1, 2020
@wltan
Add latest report from AY2021S1-CS2103T-W16-3/tp#39
88a92ae
wltan
wltan approved these changes Oct 1, 2020
View reviewed changes
Copy link
Member

@wltan wltan left a comment

Choose a reason for hiding this comment

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

LGTM!

Pitest report: https://ay2021s1-cs2103t-w16-3.github.io/reports/pitest/202010012150/

@wltan wltan merged commit aa5158c into AY2021S1-CS2103T-W16-3:master Oct 1, 2020
@wltan wltan mentioned this pull request Oct 1, 2020
Closed
@ianyong ianyong added priority.medium 🥈 Todo for current iteration and removed priority.high 🥇 Overdue changes; fixing this is critical labels Oct 1, 2020
ianyong added a commit to ianyong/tp that referenced this pull request Oct 2, 2020
@ianyong
Remove 'Address' model
c65ecae 
Also remove corresponding test class. The deletion of these classes
was silently reverted due to rebasing on AY2021S1-CS2103T-W16-3#39 and AY2021S1-CS2103T-W16-3#47.
zhaojj2209 pushed a commit that referenced this pull request Oct 2, 2020
@ianyong
Remove 'address' field from 'person' model (#48)
1c5decb 
* Remove 'address' field from 'person' model

* Remove stray `INVALID_ADDRESS` field

* Remove redundant test and update comment

* Remove 'Address' model

Also remove corresponding test class. The deletion of these classes
was silently reverted due to rebasing on #39 and #47.
@wltan wltan mentioned this pull request Oct 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ianyong ianyong ianyong approved these changes

@wltan wltan wltan approved these changes

@zhaojj2209 zhaojj2209 zhaojj2209 left review comments

Assignees
No one assigned
Labels
priority.medium 🥈 Todo for current iteration type.task 📋 Miscellaneous tasks that aren't feature or documentation-related
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@yongping827 @wltan @ianyong @zhaojj2209