Fix time zone name key by jodastephen · Pull Request #827 · JodaOrg/joda-time

jodastephen · 2026-01-21T20:16:02Z

The code '%z' is used as a time zone name key in certain simple cases It had been mapped to a zone offset in some, but not all, circumstances.
Fixes #800

Summary by CodeRabbit

Release Notes

Bug Fixes
- Fixed time zone name key formatting to prevent incorrect character returns.
Updates
- DateTimeZone reference data updated to 2025cgtz.
Tests
- Added test coverage for time zone compilation and formatting validation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-21T20:16:23Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Fixes timezone name-key generation by using Rule.formatName(...) when compiling zones so setFixedSavings() receives a formatted name instead of raw %z-style formats; adds tests and release notes for Asia/Dubai and a 2.14.1 changelog entry.

Changes

Cohort / File(s)	Summary
Release notes `src/changes/changes.xml`	Added 2.14.1 (SNAPSHOT) release block documenting fix for name key returning `%z` and DateTimeZone data update to 2025cgtz.
Zone compilation logic `src/main/java/org/joda/time/tz/ZoneInfoCompiler.java`	`addToBuilder()` now computes `formattedName` via `Rule.formatName(...)` and passes it to `builder.setFixedSavings(...)` instead of raw `zone.iFormat`; same approach applied for zones with and without RuleSet (uses parsed `saveMillis` when present).
Test data `src/test/java/org/joda/time/tz/TestBuilder.java`	Added `ASIA_DUBAI_DATA` static matrix with Dubai transition rows (LMT, +04) used in tests.
Tests `src/test/java/org/joda/time/tz/TestCompiler.java`	Added `DUBAI_FILE` TZ snippet and two tests: `testCompileDubaiPercentZ()` (compile-and-verify Asia/Dubai) and `test_Dubai()` (assert `getNameKey()` yields "+04" rather than "%z").

Sequence Diagram(s)

sequenceDiagram
    participant C as ZoneInfoCompiler
    participant Z as Zone
    participant R as Rule
    participant B as DateTimeZoneBuilder
    participant RS as RuleSet

    C->>Z: read zone (iFormat, iOffsetMillis, iRules)
    alt iRules == null
        C->>R: Rule.formatName(iFormat, iOffsetMillis, 0, null)
        R-->>C: formattedName
        C->>B: setFixedSavings(formattedName, offset, 0)
    else iRules references RuleSet
        C->>RS: parseTime(iRules) / lookup
        RS-->>C: saveMillis (or throws)
        C->>R: Rule.formatName(iFormat, iOffsetMillis, saveMillis, null)
        R-->>C: formattedName
        C->>B: setFixedSavings(formattedName, offset, saveMillis) or rs.addRecurring(...)
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description concisely explains the problem, states it fixes issue `#800`, but does not follow the repository's template which requests PR deletion upon submission.	Remove the template text from the description or provide more detail about testing and implementation approach.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix time zone name key' directly addresses the primary objective of fixing the issue where getNameKey returns '%z' for certain timezones.
Linked Issues check	✅ Passed	The code changes fix the '%z' formatting issue by modifying ZoneInfoCompiler to use Rule.formatName instead of raw iFormat, and add comprehensive test coverage for Dubai timezone data, directly addressing issue `#800`'s requirements.
Out of Scope Changes check	✅ Passed	All changes are within scope: changelog entry for release 2.14.1, core fix in ZoneInfoCompiler.addToBuilder, and test data and test cases for Dubai timezone validation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR fixes a bug where the time zone format code '%z' was incorrectly used as a literal zone name instead of being formatted as an offset. The fix applies Rule.formatName() to properly convert format codes like '%z', '/', and '%s' into their actual representations before passing them to the builder.

Changes:

Modified ZoneInfoCompiler.java to call Rule.formatName() before setting fixed savings
Added test case for Dubai timezone which uses '%z' format
Added test data structure for Asia/Dubai transitions
Updated changes.xml with fix description

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/main/java/org/joda/time/tz/ZoneInfoCompiler.java	Added calls to `Rule.formatName()` to format zone names before passing to builder
src/test/java/org/joda/time/tz/TestCompiler.java	Added Dubai test case and test data constant
src/test/java/org/joda/time/tz/TestBuilder.java	Added ASIA_DUBAI_DATA test data structure
src/changes/changes.xml	Documented the fix in version 2.14.1

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

The code '%z' is used as a name key in certain simple cases It had been mapped to a zone offset in some, but not all, circumstances Fixes #800

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/main/java/org/joda/time/tz/ZoneInfoCompiler.java`:
- Around line 1064-1068: In ZoneInfoCompiler's numeric-rules branch, the
formattedName is built with Rule.formatName(zone.iFormat, zone.iOffsetMillis, 0,
null) which passes 0 for the savings and causes %z/STD-DST formatting to be
incorrect; change the call to use the parsed saveMillis from
parseTime(zone.iRules) (i.e., Rule.formatName(zone.iFormat, zone.iOffsetMillis,
saveMillis, null)) before calling builder.setFixedSavings(formattedName,
saveMillis) so the name key reflects the actual fixed saving.

### What changes were proposed in this pull request? This PR upgrades `joda-time` to `2.14.1`. ### Why are the changes needed? To bring in the latest upstream bug fixes and keep the dependency up to date. - https://www.joda.org/joda-time/changes-report.html#a2.14.1 - https://github.com/JodaOrg/joda-time/releases/tag/v2.14.1 - JodaOrg/joda-time#827 - JodaOrg/joda-time#829 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-6) Closes #55293 from dongjoon-hyun/SPARK-56439. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org>

Copilot AI review requested due to automatic review settings January 21, 2026 20:16

Copilot started reviewing on behalf of jodastephen January 21, 2026 20:17 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

Comment thread src/main/java/org/joda/time/tz/ZoneInfoCompiler.java Outdated

Fix name key

8e8424e

The code '%z' is used as a name key in certain simple cases It had been mapped to a zone offset in some, but not all, circumstances Fixes #800

jodastephen force-pushed the fix-name-key branch from 501d3c8 to 8e8424e Compare January 21, 2026 20:27

coderabbitai Bot requested changes Jan 21, 2026

View reviewed changes

Comment thread src/main/java/org/joda/time/tz/ZoneInfoCompiler.java

coderabbitai Bot approved these changes Jan 21, 2026

View reviewed changes

jodastephen merged commit c4bf877 into main Jan 21, 2026
6 checks passed

jodastephen deleted the fix-name-key branch January 21, 2026 20:45

bf2robot mentioned this pull request Jan 6, 2026

chore(deps): update dependency joda-time:joda-time to v2.14.1 bf2fc6cc711aee1a0c2a/e2e-test-suite#507

Open

1 task

dongjoon-hyun mentioned this pull request Apr 10, 2026

[SPARK-56439][BUILD] Upgrade joda-time to 2.14.1 apache/spark#55293

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix time zone name key#827

Fix time zone name key#827
jodastephen merged 1 commit intomainfrom
fix-name-key

jodastephen commented Jan 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 21, 2026 •

edited

Loading

Other AI code review bot(s) detected

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jodastephen commented Jan 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other AI code review bot(s) detected

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

jodastephen commented Jan 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 21, 2026 •

edited

Loading