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

Update guide #258

Merged
merged 10 commits into from
Oct 24, 2020
Merged

Conversation

ZoroarkDarkrai
Copy link

@ZoroarkDarkrai ZoroarkDarkrai commented Oct 23, 2020

  • Add internship ss

  • Clear command -> ug,dg

  • Storage -> dg

  • Command summary

@ZoroarkDarkrai ZoroarkDarkrai requested review from orzymandias and a team October 23, 2020 15:07
@codecov-io
Copy link

codecov-io commented Oct 23, 2020

Codecov Report

Merging #258 into master will decrease coverage by 0.76%.
The diff coverage is 75.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #258      +/-   ##
============================================
- Coverage     51.06%   50.29%   -0.77%     
+ Complexity      851      834      -17     
============================================
  Files           197      195       -2     
  Lines          3629     3575      -54     
  Branches        400      390      -10     
============================================
- Hits           1853     1798      -55     
- Misses         1655     1656       +1     
  Partials        121      121              
Impacted Files Coverage Δ Complexity Δ
...c/main/java/seedu/address/model/person/Person.java 81.13% <0.00%> (-1.89%) 22.00 <0.00> (-1.00)
...rc/main/java/seedu/address/logic/LogicManager.java 83.33% <100.00%> (ø) 14.00 <0.00> (ø)
src/main/java/seedu/address/model/tag/Tag.java 70.00% <0.00%> (-10.00%) 4.00% <0.00%> (-1.00%)
...rc/main/java/seedu/address/model/ModelManager.java 65.65% <0.00%> (+1.01%) 39.00% <0.00%> (+1.00%)

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 6484bd6...62714eb. Read the comment docs.

@@ -263,13 +257,14 @@ private void assertCommandFailure(String inputCommand, Class<? extends Throwable
/**
* A stub class to throw an {@code IOException} when the save method is called.
*/
private static class JsonItemListIoExceptionThrowingStub extends JsonItemListStorage<Person, JsonAdaptedPerson> {

Choose a reason for hiding this comment

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

@ZoroarkDarkrai I am not sure if this class is being used? If i recall correctly it is only used in line 94, which is being removed

Copy link
Author

Choose a reason for hiding this comment

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

Idk either? I was just removing person from AB3.

Copy link
Author

Choose a reason for hiding this comment

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

better remove it? or would it be useful in the future?

Choose a reason for hiding this comment

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

Maybe we leave it here first.

Comment on lines 255 to 266
- At least one of the optional fields must be provided.

Examples:
- `edit int 7 i/1 w/2000 r/Java r/Python`
- `edit int 4 i/4 j/ML Engineer`
- Executing: `edit int 1 i/2 w/4800 r/Java r/Python` will change:

![EditInternshipBefore](images/EditInternshipBefore.png)

to:

![EditInternshipAfter](images/EditInternshipAfter.png)

- `edit int 3 i/1 j/Frontend Developer`

Choose a reason for hiding this comment

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

I think it could be better to specify a note saying that the details of the application made for this internship will also be updated accordingly.

Choose a reason for hiding this comment

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

Maybe add an image to supplement that too!

Copy link
Author

Choose a reason for hiding this comment

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

image of the application?

Comment on lines 255 to 266
- At least one of the optional fields must be provided.

Examples:
- `edit int 7 i/1 w/2000 r/Java r/Python`
- `edit int 4 i/4 j/ML Engineer`
- Executing: `edit int 1 i/2 w/4800 r/Java r/Python` will change:

![EditInternshipBefore](images/EditInternshipBefore.png)

to:

![EditInternshipAfter](images/EditInternshipAfter.png)

- `edit int 3 i/1 j/Frontend Developer`

Choose a reason for hiding this comment

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

Maybe add an image to supplement that too!

docs/UserGuide.md Outdated Show resolved Hide resolved
docs/UserGuide.md Show resolved Hide resolved
InternHunter automatically saves user data after every command. The following sequence diagram demonstrates how
InternHunter does it. Let `commandString` be any valid command string.

![SavingDataSequenceDiagram](images/SavingDataSequenceDiagram.png)

Choose a reason for hiding this comment

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

Not sure why its not rendering on github? does your image render properly locally?

Choose a reason for hiding this comment

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

Also I don't think StorageManager is in :Model?

Copy link
Author

Choose a reason for hiding this comment

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

nice catch! and it does

@seanjyjy
Copy link

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants