Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correct Swift compile errors for comparison expressions #494

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

jbelkins
Copy link
Contributor

Description of changes

When building the SDK with waiters, it was discovered that JMES compare expressions with integers and ==, and with doubles and !=, fail to compile due to ambiguity in resolving the operator.

  • The Integer / == problem is resolved by requiring a Double comparator, then converting the Ints to Double before comparison.
  • The Double / != problem is resolved by using !JMESUtils.compare(a, ==, b) instead of JMESUtils.compare(a, !=, b) when inequality is to be tested.

Unit tests have been added to this PR to test compile-time resolution of various arguments to JMESUtils.compare(_:_:_:). Generated tests for number equality & inequality expressions are being added in a separate PR.

Scope

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jbelkins jbelkins merged commit 40efc69 into feat/waiters Dec 19, 2022
@jbelkins jbelkins deleted the jbe/jmes_comparison_fixes branch December 19, 2022 17:58
jbelkins added a commit that referenced this pull request Dec 19, 2022
* feat: Add waiter client runtime components (#450)
* feat: Waiter extension & methods (#478)
* feat: Add WaiterTypedError type, conform operation errors to it (#491)
* feat: Code-generate Acceptors for waiters (#483)
* fix: Use correct var name in AND expression (#493)
* fix: Correct Swift compile errors for comparison expressions (#494)
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