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

Provide masks DDD and DDDD to print "Yesterday", "Today" or "Tomorrow". #71

Merged
merged 12 commits into from Dec 22, 2020

Conversation

TalhaAwan
Copy link
Contributor

  • If the date belongs to yesterday, today or tomorrow the masks should print so.
  • If the date does not lie in these days then DDD should fall back to ddd and DDDD to dddd.
  • Unit tests added
  • Readme updated

@TalhaAwan
Copy link
Contributor Author

Though tests are passing, the logic is incorrect. I mistakenly used day of week instead of day of month. Even with day of month, boundary conditions on changing months and years will need to be handled. Will try to work on this later.

@TalhaAwan
Copy link
Contributor Author

Fixed with simplified logic.

@chase-manning
Copy link
Collaborator

@TalhaAwan Thanks so much for this PR, is a great idea!!!
Sorry this was not merged sooner, this repo was not under active maintenance at the time.
I would love to merge this PR but it has merge conflicts at the moment.
Would you be able to please fix the merge conflicts so I can merge this?

Copy link
Collaborator

@chase-manning chase-manning left a comment

Choose a reason for hiding this comment

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

Thanks so much for the PR!!!
I have added some review comments to be resolved.

Readme.md Outdated Show resolved Hide resolved
lib/dateformat.js Outdated Show resolved Hide resolved
@TalhaAwan
Copy link
Contributor Author

@chase-manning I resolved the readme conflicts only to find out that tests are not passing. Lots have changed since I opened this PR, so I'll have to re-add this functionality. Will do that soon.

Copy link
Collaborator

@chase-manning chase-manning left a comment

Choose a reason for hiding this comment

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

Looks great!!! ❤️

@chase-manning chase-manning merged commit fa071e5 into felixge:master Dec 22, 2020
@TalhaAwan
Copy link
Contributor Author

@chase-manning "Yes", "Tom" and "Tod" look odd though. Do you think we should change that? I was looking it up and YDA, TDA, and TMW/TMR are the abbreviations that are used in certain fields. Let me know if this needs change.

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

Successfully merging this pull request may close these issues.

None yet

2 participants