Skip to content

Deserialize timeZoneName while considering known aliases. - #1200

Merged
rma-rripken merged 4 commits into
developfrom
feature/1192-zonename
Aug 4, 2025
Merged

Deserialize timeZoneName while considering known aliases.#1200
rma-rripken merged 4 commits into
developfrom
feature/1192-zonename

Conversation

@rma-rripken

Copy link
Copy Markdown
Collaborator

No description provided.

@MikeNeilson MikeNeilson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Conceptually this looks reasonable. Wish there was a better way to handle the aliases... would just doing a .json file be cleaner than trying to map to the java properties syntax?

* values are their corresponding standard timezone IDs
*/
public static @NotNull Map<String, String> buildTimeZoneAliases() {
Map<String, String> aliases = buildResourceAliases();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would it be better to just load the defaults into the map, and then let the resource file override any existing entries? versus the all or nothing approach?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You could be right. My worry was that there would be no good way to replace a default if the default was wrong or no longer wanted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It weirded me out to need jackson to read the resource when the class was going to be needed by jackson objectmappers. Idk, maybe it would just work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... fair point. though the most generic instance of ObjectMapper would work.

But there's also nothing wrong with the current setup, just also seemed... awkward.

@rma-rripken
rma-rripken marked this pull request as ready for review August 1, 2025 16:36
@rma-rripken
rma-rripken merged commit 2c19ffc into develop Aug 4, 2025
7 checks passed
@rma-rripken
rma-rripken deleted the feature/1192-zonename branch August 4, 2025 16:09
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.

2 participants