-
Notifications
You must be signed in to change notification settings - Fork 5
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
Branch kk filter articles by date #70
Branch kk filter articles by date #70
Conversation
Add ArticleMatchesStatusPredicate class. Add exception for invalid statuses.
SetArticleCommandParser
new method getFilter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, seem to have a lot of overlap with #66, I'm not sure whether order of merging will be relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I right to assume this PR should be merged before #66 since the command wouldn't work without the this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think filter by status should be merged first. This is an extension of that pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job fixing the comments
Add ability to filter by date.
Command format is set -a S/DRAFT ST/ EN/