-
-
Notifications
You must be signed in to change notification settings - Fork 383
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
feat: Added "--after" option #52
base: master
Are you sure you want to change the base?
Conversation
@@ -235,6 +238,16 @@ class Taskbook { | |||
return data; | |||
} | |||
|
|||
_filterAfterDate(date = moment().format(DATE_FORMATTER), data = this._data) { | |||
Object.entries(data).forEach(([id, item]) => { |
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.
Can this be re-written as the following?
Object.entries(data).filter(([id, item]) => {
return moment(item._timestamp).isAfter(moment(date))
});
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.
Yes, the filter
is better than foreach
for this case. Also deleting object is a little bit time expensive
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.
sorry, I can NOT, because the data
is not an array, and I have to change the data directy in this project.
We should probably update the help documentation as well. My understanding is that any ISO-8601 format should be parsed (https://momentjs.com/docs/#/parsing/string/) with your PR; not just the "20180806" format, which is pretty dang nifty! |
I think without using |
I would suggest using date-fns it is modular and its perf is better https://medium.com/@k2u4yt/momentjs-vs-date-fns-6bddc7bfa21e |
It looks like date-fns also supports ISO-8601, so that's cool too. I think it's good to keep perf and size in mind, but it's worth mentioning that ~50kb is still fairly small when it comes to a globally downloaded node module like this that isn't being passed up and down the wire with every request. Moment has a bit more usage and support than date-fns right now, but it looks like date-fns is being iterated on a bit more frequently than moment is. |
I agree. I also think performance wise date-fns is better and I think that's important since tb takes some time to process operations |
That's a good idea, I will replace moment with date-fns. @knappg |
We can use
--after
to filter the task data by date. like this:or