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 context swinging #77

Merged
merged 17 commits into from
Mar 19, 2019
Merged

Add context swinging #77

merged 17 commits into from
Mar 19, 2019

Conversation

epicfailname
Copy link

@epicfailname epicfailname commented Mar 17, 2019 •
edited

Resolves #79
Resolves part of #69 and #70.

Summary of changes

  • Add stubs for future archive and unarchive commands
  • Implement context-swinging to support future feed-listing and archiving features
    • Model is now tied to a ModelContext
    • Only allows a limited set of commands (out of all available commands in the app) per context
    • User can switch contexts by calling the context-switching commands

@epicfailname
Add state for LogicManager class
a3f4fee
@epicfailname
Add stub for archives command
88031f0
@epicfailname
Move states from Logic to Model
6230b9e 
Refactor state -> context
@epicfailname
Update parser architecture, support context switch
d923df0 
Add stubs for archives context
@epicfailname
Add tests for parser for archives context
182684f
@epicfailname
Rename assertion for status bar to exclude count
37376ad
@epicfailname
Add stubs for Archvive and Unarchive commands
da3e8eb
@epicfailname
Add enum class for Model contexts
bbf2e49
@epicfailname
Add stub for archive/unarchive entry in Model
45a68c3
@epicfailname
Update clone() to clone the context as well
9f1fd84
@epicfailname
Update getModel() to use clone() in Model
c2561ff 
Behavior should be the same, both returns a defensive copy of Model
@epicfailname
Add equals() method for Archive/UnarchiveCommand
277ff7d
@epicfailname
Update regression tests for context-switching
907a02a 
Add system test for context-switching
@epicfailname
Revert previous commit
2cc45d2 
Realised that using clone() is probably not defensive since Storage and UserPrefs cloned along with EntryBook
@epicfailname epicfailname force-pushed the add-context-swinging branch 2 times, most recently from e23721c to 0513883 Compare March 17, 2019 16:25
@coveralls

This comment has been minimized.

Sign in to view
qjqqyy
qjqqyy reviewed Mar 17, 2019
View reviewed changes
Copy link

@qjqqyy qjqqyy left a comment

Choose a reason for hiding this comment

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

Great work! Only thing I'm still not too sure about is how best to communicate the current context to the user.

src/main/java/seedu/address/logic/commands/ArchivesCommand.java Show resolved Hide resolved
src/main/java/seedu/address/logic/LogicManager.java Outdated Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/EntryBookArchivesParser.java Show resolved Hide resolved
src/main/java/seedu/address/logic/parser/EntryBookListParser.java Show resolved Hide resolved
src/test/java/systemtests/AddCommandSystemTest.java Show resolved Hide resolved
@qjqqyy qjqqyy added this to the v1.2 milestone Mar 18, 2019
qjqqyy
qjqqyy approved these changes Mar 18, 2019
View reviewed changes
Copy link

@qjqqyy qjqqyy left a comment

Choose a reason for hiding this comment

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

It's only natural for coverage to drop through such huge refactors.

@qjqqyy qjqqyy mentioned this pull request Mar 18, 2019
Merged
@epicfailname epicfailname merged commit 8a25efc into master Mar 19, 2019
@qjqqyy qjqqyy deleted the add-context-swinging branch March 19, 2019 09:26
@thomastanck thomastanck mentioned this pull request Mar 19, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomastanck thomastanck thomastanck left review comments

@qjqqyy qjqqyy qjqqyy approved these changes

@rlrh rlrh rlrh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@epicfailname @coveralls @thomastanck @qjqqyy @rlrh