Skip to content

Conversation

@WellerQu
Copy link

@WellerQu WellerQu commented Aug 4, 2018

We can use --after to filter the task data by date. like this:

tb --list done --after 20180804

or

tb --after 20180804

}

_filterAfterDate(date = moment().format(DATE_FORMATTER), data = this._data) {
Object.entries(data).forEach(([id, item]) => {
Copy link

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))
});

Copy link

@rjoydip-zz rjoydip-zz Aug 7, 2018

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

Copy link
Author

@WellerQu WellerQu Aug 8, 2018

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.

@knappg
Copy link

knappg commented Aug 7, 2018

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!

@klaudiosinani klaudiosinani self-requested a review August 7, 2018 16:18
@klaudiosinani klaudiosinani added the enhancement New feature or request label Aug 7, 2018
@rjoydip-zz
Copy link

rjoydip-zz commented Aug 7, 2018

I think without using moment this could be possible in native way. Moment has 48-50kb lib size.

@ashinzekene
Copy link
Contributor

I would suggest using date-fns it is modular and its perf is better

https://medium.com/@k2u4yt/momentjs-vs-date-fns-6bddc7bfa21e

@knappg
Copy link

knappg commented Aug 8, 2018

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.

@ashinzekene
Copy link
Contributor

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

@WellerQu
Copy link
Author

WellerQu commented Aug 8, 2018

That's a good idea, I will replace moment with date-fns. @knappg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants