se-edu / addressbook-level3

:ab::three: Address Book sample application (Level 3)
https://se-education.org/addressbook-level3
MIT License
28 stars 415 forks source link

JsonAdaptedPerson class: Change `tagged` to `tags` #153

Closed Py0000 closed 1 year ago

Py0000 commented 1 year ago

Fixes #147

Change the variable tagged to tags in the affected class (src/main/java/seedu/address/storage/JsonAdaptedPerson.java). Re-opened as a new PR.

canihasreview[bot] commented 1 year ago

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

codecov-commenter commented 1 year ago

Codecov Report

Merging #153 (64f1e54) into master (412b5ed) will not change coverage. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff            @@
##             master     #153   +/-   ##
=========================================
  Coverage     72.15%   72.15%           
  Complexity      399      399           
=========================================
  Files            70       70           
  Lines          1232     1232           
  Branches        125      125           
=========================================
  Hits            889      889           
  Misses          311      311           
  Partials         32       32           
Impacted Files Coverage Δ
.../java/seedu/address/storage/JsonAdaptedPerson.java 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

canihasreview[bot] commented 1 year ago

v1

@Py0000 submitted v1 for review.

(:books: Archive)

Checkout this PR version locally ```sh git fetch https://github.com/se-edu/addressbook-level3.git refs/pr/153/1/head:BRANCHNAME ``` where `BRANCHNAME` is the name of the local branch you wish to fetch this PR to.
Py0000 commented 1 year ago

Hi Prof @damithc, I have closed the previous PR #148 for this issue (#147) and re-opened it as a new PR over here as I might have accidentally screwed up my own local repository. Sorry for the inconvenience regarding this.

damithc commented 1 year ago

Also, we need a full commit message as per our usual practice.

Py0000 commented 1 year ago

Also, we need a full commit message as per our usual practice.

Sorry about that Prof. I will go and figure out how to edit the commit message.

Py0000 commented 1 year ago

I have updated the commit message.

damithc commented 1 year ago
@JsonProperty("tagged") List<JsonAdaptedTag> tagged) {

I think it is best if we modify both occurrences of tagged, if possible (or else we introduce an inconsistency which can be argued as even worse than the naming anomaly being fixed). So, I would like to explore that option before merging this PR.

damithc commented 1 year ago

Fixed by #169 that builds on this PR. Thanks again for this PR @Py0000