Additional wordBased method on AmountFormats#373
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit 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. Caution Review failedThe pull request is closed. WalkthroughExpanded CI triggers to include push, pull_request, and schedule. Added a new AmountFormats overload to format PeriodDuration in a word-based manner by delegating to existing Period and Duration formatters. Tests updated to cover PeriodDuration and added locale constants for Romanian and Persian. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant AmountFormats
participant PeriodDuration
participant WordFormatter as Existing wordBased(period/duration)
Caller->>AmountFormats: wordBased(PeriodDuration pd, Locale loc)
AmountFormats->>PeriodDuration: getPeriod()
AmountFormats->>WordFormatter: wordBased(period, loc)
AmountFormats->>PeriodDuration: getDuration()
AmountFormats->>WordFormatter: wordBased(duration, loc)
AmountFormats-->>Caller: concatenated word-based string
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8–10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new convenience method to AmountFormats for formatting PeriodDuration objects directly in word-based format. Previously, users had to extract the period and duration components separately to use the existing wordBased method.
- Adds a new
wordBasedmethod that accepts aPeriodDurationobject and locale - Refactors test code to use constants for locale instances instead of inline
new Locale()calls - Updates documentation to include
PeriodDurationin the ISO-8601 format method description
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/java/org/threeten/extra/AmountFormats.java | Adds new wordBased(PeriodDuration, Locale) method and updates documentation |
| src/test/java/org/threeten/extra/TestAmountFormats.java | Adds locale constants and test assertion for the new method |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| * @param locale the locale to use | ||
| * @return the localized word-based format for the period-duration | ||
| */ | ||
| public static String wordBased(PeriodDuration periodDuration, Locale locale) { |
There was a problem hiding this comment.
The method should include null parameter validation for both periodDuration and locale parameters to maintain consistency with the existing wordBased method behavior and prevent potential NullPointerExceptions.
| public static String wordBased(PeriodDuration periodDuration, Locale locale) { | |
| public static String wordBased(PeriodDuration periodDuration, Locale locale) { | |
| Objects.requireNonNull(periodDuration, "periodDuration"); | |
| Objects.requireNonNull(locale, "locale"); |
Allow `PeriodDuration` to be directly formatted Also remove trailing whitespace in yaml See #269
2539786 to
1e5e529
Compare
Allow
PeriodDurationto be directly formattedAlso remove trailing whitespace in yaml
See #269
Summary by CodeRabbit