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

[Issue #2736] refactor dateUtils to use dayJS #3892

Merged
merged 18 commits into from
Feb 21, 2025

Conversation

karinamzalez
Copy link
Contributor

@karinamzalez karinamzalez commented Feb 13, 2025

Summary

Fixes #2736

Time to review: 5 mins

Changes proposed

What was added, updated, or removed in this PR.

  • updates dateUtils > formatDate to utilize dayJS
  • Specifies node version in README instructions.

Context for reviewers

Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers. Explain how the changes were verified.

  • dayJS is now utilized to validate and format dates

Additional information

Screenshots, GIF demos, code examples or output to help show the changes working as expected.
Screenshot 2025-02-14 at 12 55 50 PM

@karinamzalez
Copy link
Contributor Author

QUESTION: I kept the dateUtils formatDate function / refactored it instead of removing it completely to keep some uniform date string validation prior to formatting. The original ticket does state that I should deprecate this function entirely, should I disregard the potential need for date string validation and fully deprecate this dateUtils function?

@karinamzalez karinamzalez changed the title 2736 add dayjs [Issue #2736] refactor dateUtils to use dayJS Feb 14, 2025
@mdragon
Copy link
Collaborator

mdragon commented Feb 14, 2025

should I disregard the potential need for date string validation and fully deprecate this dateUtils function?

Thanks for working on this, @karinamzalez! I think you're identifying value in this function remaining, that likely just hadn't been thought of when we created the ticket, so I think we can tweak the ticket to adapt to your suggestion.

Copy link
Collaborator

@acouch acouch 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, thanks.

@btabaska btabaska merged commit 8dcddc9 into HHS:main Feb 21, 2025
8 checks passed
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.

Tech Debt: Use the newly added day.js library to replace our existing dateUtils function
4 participants