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.

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

Storage v1 #24

Merged
merged 23 commits into from
Oct 12, 2020
Merged

Storage v1 #24

merged 23 commits into from
Oct 12, 2020

Conversation

Nahoyhp
Copy link

@Nahoyhp Nahoyhp commented Oct 6, 2020

Read, Write and Archive for ExerciseBook is completed.

- Check read and write is complete.
@Nahoyhp Nahoyhp added this to the V1.2 milestone Oct 6, 2020
@Nahoyhp Nahoyhp self-assigned this Oct 6, 2020
Copy link

@leeweiminsg leeweiminsg left a comment

Choose a reason for hiding this comment

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

Tested locally, default test cases passed.

Issue when running TestDriver.java locally:

12:14:17 PM: Executing task 'TestDriver.main()'...

Task :compileJava UP-TO-DATE
Task :processResources UP-TO-DATE
Task :classes UP-TO-DATE

Task :TestDriver.main()
Exceptions

BUILD SUCCESSFUL in 0s
3 actionable tasks: 1 executed, 2 up-to-date
Oct 07, 2020 12:14:18 PM seedu.address.commons.util.JsonUtil readJsonFile
INFO: Json file data\testing.json not found
12:14:18 PM: Task execution finished 'TestDriver.main()'.

Data stored in the /data/ won't be included in commit (listed in gitignore).

Perhaps you can move them to src/test/data instead?
Also, I think the TestDriver class should belong somewhere in src/test instead. Classes in src/main are used in running the app itself.

@Nahoyhp
Copy link
Author

Nahoyhp commented Oct 7, 2020

This is not my final copy yet. Just want to run the above tests to see where I need to help.

@Nahoyhp Nahoyhp marked this pull request as draft October 8, 2020 15:27
…nto v1.1-storage

# Conflicts:
#	docs/DeveloperGuide.md
#	docs/UserGuide.md
…nto v1.1-storage

# Conflicts:
#	docs/DeveloperGuide.md
#	docs/UserGuide.md
…nto v1.1-storage

# Conflicts:
# docs/DeveloperGuide.md
# docs/UserGuide.md
…to Parser<T extends Command>

Potential Bugs
ArchiveCommandParser should extends Parser<CommandForExercise>
@Nahoyhp Nahoyhp removed the type.story label Oct 8, 2020
@Nahoyhp Nahoyhp linked an issue Oct 8, 2020 that may be closed by this pull request
@codecov-io
Copy link

codecov-io commented Oct 8, 2020

Codecov Report

Merging #24 into v1.2 will decrease coverage by 8.90%.
The diff coverage is 32.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##               v1.2      #24      +/-   ##
============================================
- Coverage     72.31%   63.40%   -8.91%     
- Complexity      400      451      +51     
============================================
  Files            70       90      +20     
  Lines          1228     1585     +357     
  Branches        124      156      +32     
============================================
+ Hits            888     1005     +117     
- Misses          308      538     +230     
- Partials         32       42      +10     
Impacted Files Coverage Δ Complexity Δ
src/main/java/seedu/address/TestDriver.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...a/seedu/address/logic/LogicManagerForExercise.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...a/seedu/address/logic/commands/ArchiveCommand.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...edu/address/logic/commands/CommandForExercise.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...edu/address/logic/parser/ArchiveCommandParser.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...seedu/address/logic/parser/ExerciseBookParser.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...in/java/seedu/address/logic/parser/ParserUtil.java 76.08% <0.00%> (-21.14%) 14.00 <0.00> (ø)
...c/main/java/seedu/address/model/ExerciseModel.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ain/java/seedu/address/model/ModelForExercise.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...va/seedu/address/storage/JsonUserPrefsStorage.java 63.63% <0.00%> (-23.87%) 4.00 <0.00> (ø)
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8a6f84...b5cb56d. Read the comment docs.

@Nahoyhp Nahoyhp marked this pull request as ready for review October 9, 2020 16:15
@Liu-2001
Copy link

Liu-2001 commented Oct 10, 2020

Tested locally. All storage related tests passed.

For TestDriver.java, if I run it multiple times, the data will be stored repeatedly into testingForExercise.json. Is that the desired outcome or should you first clear the file and then store the exercise book? (not sure about this)

Otherwise LGTM.

@Nahoyhp
Copy link
Author

Nahoyhp commented Oct 11, 2020

The TestDriver is created to ensure that I can read and write from the data.
The reason for the repeats is because it reads all the items in the storage, can't realize that items with same name and date are the same (cuz i am using stubs) and just add items at the end of the list.
The TestDriver also includes reading from past records and add the same set of Exercise at the end.

@Nahoyhp Nahoyhp merged commit 1e00a23 into AY2021S1-CS2103T-W17-2:v1.2 Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As a user, I can save my data in a file
4 participants