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

Add new date format pattern to DCDate #9253

Merged
merged 1 commit into from Apr 23, 2024

Conversation

kshepherd
Copy link
Member

@kshepherd kshepherd commented Jan 11, 2024

Description

This small change adds an additional date format to DCDate for parsing purposes: yyyy-MM-dd'T'HH:mm:ss'.000'
This date format is used by some APIs and is useful to have in our list of formats to support when parsing date strings.

Instructions for Reviewers

Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.

List of changes in this PR:

  • New pattern added to DCDate, and included in this class's methods where all patterns are tried or added in succession
  • New tests added to DCDateTest to ensure date parsing is successful for the new format
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
  • My PR passes Checkstyle validation based on the Code Style Guide.
  • My PR passes all tests and includes new/updated Unit or Integration Tests based on the Code Testing Guide.

@kshepherd kshepherd added improvement quick win Pull request is small in size & should be easy to review and/or merge labels Jan 11, 2024
@kshepherd kshepherd added this to the 8.0 milestone Jan 11, 2024
@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge. label Jan 11, 2024
@nwoodward
Copy link
Contributor

The code changes make complete sense, but is there a way to test them? It appears I can use full ISO dates with milliseconds in date fields now, but I'm guessing some functionality is missing.

@kshepherd
Copy link
Member Author

@nwoodward I added the patterns to DCDateTest (it is one of the files changed by this PR), so that is my test or "proof". As to where this new format / pattern is actually used in DSpace -- it isn't yet, I just wanted to get this small change included now so that future work which does require milliseconds is able to be merged more easily.

@tdonohue tdonohue self-requested a review April 11, 2024 14:49
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @kshepherd . This seems harmless and has tests. Merging immediately.

@tdonohue tdonohue merged commit 29d9172 into DSpace:main Apr 23, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge. improvement quick win Pull request is small in size & should be easy to review and/or merge
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants