Skip to content

Conversation

@rismay
Copy link
Member

@rismay rismay commented Aug 13, 2025

Summary

  • cover URL query item builders for arrays, dictionaries, and numeric dictionaries
  • verify URL.tempDirectory produces unique paths
  • check localized integer, double, and dollar formatting utilities
  • fix obsolete logging call and update number formatter rounding/currency prefix

Testing

  • swift test
  • swift build --target WrkstrmFoundationTests

https://chatgpt.com/codex/tasks/task_e_689c0747df6c833389166144f5960fd7

@rismay rismay requested a review from Copilot August 13, 2025 03:49

This comment was marked as outdated.

@rismay rismay requested a review from Copilot August 13, 2025 04:04
Copy link
Contributor

Copilot AI left a comment

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 adds comprehensive test coverage for URL helpers and localized formatting utilities, while also fixing an obsolete logging API call. The changes enhance test coverage for URL query item builders, temporary directory generation, and number formatting functionality.

  • Added test suites for URL query item construction from arrays and dictionaries
  • Added tests for URL temporary directory uniqueness and localized number formatting
  • Updated deprecated logging API call and improved number formatter configuration

Reviewed Changes

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

Show a summary per file
File Description
Tests/WrkstrmNetworkingTests/WrkstrmNetworkingTests.swift Updates deprecated logging API call
Tests/WrkstrmFoundationTests/URLTempDirectoryTests.swift Adds tests for URL temporary directory uniqueness
Tests/WrkstrmFoundationTests/URLQueryItemTests.swift Adds tests for URL query item builders with arrays and dictionaries
Tests/WrkstrmFoundationTests/LocalizedValuesTests.swift Adds tests for localized integer, double, and currency formatting
Sources/WrkstrmFoundation/Extensions/NumberFormatter+Localize.swift Improves number formatter configuration and documentation

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.

let previous = NumberFormatter.dollar.locale
NumberFormatter.dollar.locale = Locale(identifier: "en_US_POSIX")
defer { NumberFormatter.dollar.locale = previous }
#expect(1234.5.dollarString() == "$\u{00a0}1,234.50")
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

[nitpick] The Unicode escape sequence \u{00a0} (non-breaking space) makes the test harder to read and maintain. Consider using a more explicit approach like defining a constant or using the actual character.

Suggested change
#expect(1234.5.dollarString() == "$\u{00a0}1,234.50")
#expect(1234.5.dollarString() == "$\(NBSP)1,234.50")

Copilot uses AI. Check for mistakes.
formatter.numberStyle = .decimal
formatter.minimumFractionDigits = 2
formatter.maximumFractionDigits = 2
formatter.roundingIncrement = 0.01
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

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

Setting roundingIncrement to 0.01 may not be appropriate for all currencies. Some currencies have different decimal places (e.g., Japanese Yen has 0, some Middle Eastern currencies have 3). Consider making this configurable or using the currency's default decimal places.

Suggested change
formatter.roundingIncrement = 0.01
formatter.roundingIncrement = pow(10, -Double(formatter.maximumFractionDigits))

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants