Skip to content

Allow to deserialize more valid RFC3339 date-time and make the format customizable#5384

Merged
manusa merged 1 commit intofabric8io:mainfrom
andreaTP:issue-5382
Aug 11, 2023
Merged

Allow to deserialize more valid RFC3339 date-time and make the format customizable#5384
manusa merged 1 commit intofabric8io:mainfrom
andreaTP:issue-5382

Conversation

@andreaTP
Copy link
Copy Markdown
Member

Description

Fix #5382

Given the fact that the specification allows for even more special cases (e.g. using lower-case 't' and 's'), here we propose to cover the most common formats with defaults, but we enable the users to customize (separately) the format for serialization and deserialization of date-time fields.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@Parameter(property = "fabric8.java-generator.enum-uppercase", required = false)
Boolean enumUppercase = null;

/**
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a leftover, not used around hence not a breaking change.

@andreaTP
Copy link
Copy Markdown
Member Author

Ideally, this fix should be backported to a 6.8.1 release, cc. @manusa .

@manusa manusa added this to the 6.8.1 milestone Aug 11, 2023
Copy link
Copy Markdown
Contributor

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

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

HI @andreaTP - I've gone through the specification and checked the implementation changes, which LGTM. I dropped one very minor question about a potential formatting typo in the README, but approving anyway.

Comment thread doc/java-generation-from-CRD.md
@sonarqubecloud
Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

51.1% 51.1% Coverage
8.6% 8.6% Duplication

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@andreaTP
Copy link
Copy Markdown
Member Author

For the records:

  • the original formatting was working fine on Java 17+ but it was failing on Java 8 ( 🤯 )
  • I improved the DateTimeFormat patterns and found a cross-compatible encoding
  • I added more negative tests to more accurately verify that the output is expected

all in all, I'm happy to make this configurable so that users will be able to tweak it depending on their environment/use-case.

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.

[java-generator] date-time doesn't parse valid RFC 3339 timestamps

4 participants