Skip to content

refactor date parsing to use ISO_DATE_TIME#132

Merged
lwluc merged 1 commit into
mainfrom
parse-iso-date-times-from-feel
Feb 7, 2026
Merged

refactor date parsing to use ISO_DATE_TIME#132
lwluc merged 1 commit into
mainfrom
parse-iso-date-times-from-feel

Conversation

@maxbehr801

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors date parsing in the CarbonReductorVariableMapper to use Java's built-in ISO_DATE_TIME formatter instead of a custom date pattern. The change simplifies the code and provides more flexibility in accepting ISO 8601 date-time formats with variable millisecond precision.

Changes:

  • Replaced custom date pattern yyyy-MM-dd'T'HH:mm:ss.SSSX'['z']' with standard ISO_DATE_TIME formatter
  • Simplified error message to remove specific format details
  • Added parameterized test to verify parsing of ISO date-time strings with varying millisecond precision

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
CarbonReductorVariableMapper.java Replaced custom date pattern with ISO_DATE_TIME formatter and simplified error message
CarbonReductorVariableMapperTest.java Added parameterized test for variable-precision ISO date-time formats and updated error message assertion

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DATE_TIME_PATTERN.replace("'", "")
)
);
throw new IllegalArgumentException("Milestone: Unknown date time format");

Copilot AI Feb 6, 2026

Copy link

Choose a reason for hiding this comment

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

The error message has been made less helpful by removing the expected format. While the old message included the specific expected format pattern, the new message provides no guidance to users on what format is expected. Consider adding information about the expected ISO 8601 format or providing an example in the error message to help users correct their input.

Suggested change
throw new IllegalArgumentException("Milestone: Unknown date time format");
throw new IllegalArgumentException("Milestone: Unknown date time format. Expected ISO 8601, e.g. '2024-01-31T13:45:00Z'.");

Copilot uses AI. Check for mistakes.

@lwluc lwluc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copilot findings are useful and should be resolved

@maxbehr801 maxbehr801 force-pushed the parse-iso-date-times-from-feel branch from 215d597 to 5b4d411 Compare February 7, 2026 10:16
@maxbehr801 maxbehr801 force-pushed the parse-iso-date-times-from-feel branch from 5b4d411 to c1f5c34 Compare February 7, 2026 10:34
@maxbehr801 maxbehr801 requested a review from lwluc February 7, 2026 11:51
@lwluc lwluc merged commit 11b7c2f into main Feb 7, 2026
2 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.

3 participants