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.

Sign up for GitHub

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

Jump to bottom

@EnsureNonEmptyArrayDecodable #2831

Merged
merged 1 commit into from
Jul 18, 2023
Merged

@EnsureNonEmptyArrayDecodable #2831

merged 1 commit into from
Jul 18, 2023

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented Jul 17, 2023 •
edited
Loading

This allows us to define properties in a Decodable type and ensure that they're not empty.
Example

struct Data: Codable {
    @EnsureNonEmptyArrayDecodable
    var value: [String]
}

Used for #2832.

@NachoSoto
@EnsureNonEmptyArrayDecodable
f073691 
This allows us to define properties in a `Decodable` type and ensure that they're not empty.
Example
```swift
struct Data: Codable, Equatable {
    @EnsureNonEmptyArrayDecodable
    var value: [String]
}
```
@NachoSoto NachoSoto requested a review from a team July 17, 2023 22:12
@NachoSoto NachoSoto mentioned this pull request Jul 17, 2023
Merged
@codecov
Copy link

codecov bot commented Jul 17, 2023 •
edited
Loading

Codecov Report

Merging #2831 (f073691) into main (0ebe629) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2831      +/-   ##
==========================================
+ Coverage   86.56%   86.58%   +0.02%     
==========================================
  Files         216      217       +1     
  Lines       15495    15513      +18     
==========================================
+ Hits        13413    13432      +19     
+ Misses       2082     2081       -1
Impacted Files Coverage Δ
...es/Misc/Codable/EnsureNonEmptyArrayDecodable.swift 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

tonidero
tonidero approved these changes Jul 18, 2023
View reviewed changes
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Looks good, just a suggestion

Sources/Misc/Codable/EnsureNonEmptyArrayDecodable.swift
let array = try container.decode([Value].self)

if array.isEmpty {
throw Error()
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a bit of a generic error... Would it be possible to include the property that is causing this error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It’s EnsureNonEmptyArray.Error. When you decode the top level object JSONDecoder will include the information for what key failed :)

@NachoSoto NachoSoto merged commit 04ef06e into main Jul 18, 2023
2 checks passed
@NachoSoto NachoSoto deleted the ensure-non-empty-array branch July 18, 2023 13:12
NachoSoto added a commit that referenced this pull request Jul 18, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
538edbf 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
@RCGitBot RCGitBot mentioned this pull request Jul 19, 2023
Merged
NachoSoto added a commit that referenced this pull request Jul 19, 2023
@RCGitBot @NachoSoto
[AUTOMATIC] Release/4.25.1 (#2842)
23821af 
**This is an automatic release.**

### Dependency Updates
* Bump fastlane from 2.213.0 to 2.214.0 (#2824) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* `MainThreadMonitor`: don't crash if there is no test in progress
(#2838) via NachoSoto (@NachoSoto)
* `CI`: fixed Fastlane APITester lanes (#2836) via NachoSoto
(@NachoSoto)
* `Integration Tests`: workaround Swift runtime crash (#2826) via
NachoSoto (@NachoSoto)
* `@EnsureNonEmptyArrayDecodable` (#2831) via NachoSoto (@NachoSoto)
* `iOS 17`: added tests for simulating cancellations (#2597) via
NachoSoto (@NachoSoto)
* `CI`: make all `Codecov` jobs `informational` (#2828) via NachoSoto
(@NachoSoto)
* `MainThreadMonitor`: check deadlocks only ever N seconds (#2820) via
NachoSoto (@NachoSoto)
* New `@NonEmptyStringDecodable` (#2819) via NachoSoto (@NachoSoto)
* `MockDeviceCache`: avoid using real `UserDefaults` (#2814) via
NachoSoto (@NachoSoto)
* `throwAssertion`: fixed Xcode 15 compilation (#2813) via NachoSoto
(@NachoSoto)
* `CustomEntitlementsComputation`: fixed API testers (#2815) via
NachoSoto (@NachoSoto)
* `PackageTypeTests`: fixed iOS 12 (#2807) via NachoSoto (@NachoSoto)
* `Tests`: avoid race-condition in leak detection (#2806) via NachoSoto
(@NachoSoto)
* Revert "`Unit Tests`: removed leak detection" (#2805) via NachoSoto
(@NachoSoto)
* `PackageType: Codable` implementation (#2797) via NachoSoto
(@NachoSoto)
* `SystemInfo.init` no longer `throws` (#2803) via NachoSoto
(@NachoSoto)
* `Trusted Entitlements`: add support for signing `POST` body (#2753)
via NachoSoto (@NachoSoto)
* `Tests`: unified default timeouts (#2801) via NachoSoto (@NachoSoto)
* `Tests`: removed forced-unwrap (#2799) via NachoSoto (@NachoSoto)
* `Tests`: added missing `super.setUp()` (#2804) via NachoSoto
(@NachoSoto)
* Replaced `FatalErrorUtil` with `Nimble` (#2802) via NachoSoto
(@NachoSoto)
* `Tests`: fixed another flaky test (#2795) via NachoSoto (@NachoSoto)
* `TimingUtil`: improved tests by using `Clock` (#2794) via NachoSoto
(@NachoSoto)
* `IgnoreDecodeErrors`: log decoding error (#2778) via NachoSoto
(@NachoSoto)
* `TestLogHandler`: changed all tests to explicitly deinitialize it
(#2784) via NachoSoto (@NachoSoto)
* `LocalReceiptParserStoreKitTests`: fixed flaky test failure (#2785)
via NachoSoto (@NachoSoto)
* `Unit Tests`: removed leak detection (#2792) via NachoSoto
(@NachoSoto)
* `Tests`: fixed another flaky failure with asynchronous check (#2786)
via NachoSoto (@NachoSoto)

---------

Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
NachoSoto added a commit that referenced this pull request Jul 20, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
e347e88 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Jul 23, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
7cff9c1 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Jul 24, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
360a41e 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Jul 24, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
73811b8 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Jul 25, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
09bd2c0 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Jul 26, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
f047c58 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Jul 27, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
403443d 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Jul 31, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
de5321d 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Aug 3, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
7a10654 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Aug 7, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
678110b 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Aug 9, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
126cdc1 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Aug 11, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
ae0d59a 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Aug 14, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
abd7fa3 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Aug 17, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
3852974 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Aug 24, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
83b0a5d 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
@NachoSoto NachoSoto mentioned this pull request Aug 26, 2023
Closed
NachoSoto added a commit that referenced this pull request Aug 28, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
52e3f30 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
@NachoSoto NachoSoto mentioned this pull request Aug 31, 2023
Closed
NachoSoto added a commit that referenced this pull request Aug 31, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
1d0d641 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 1, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
e8ea78b 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 6, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
3e62cd9 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 6, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
7467031 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 6, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
d633176 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 7, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
c6e4624 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 8, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
57a4be2 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 14, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
89040bb 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 14, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
8cbd129 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
NachoSoto added a commit that referenced this pull request Sep 15, 2023
@NachoSoto
Paywalls: add support for multiple images in template configuration (…
1b8e915 
…#2832)

Changed `PaywallData.Configuration.headerImageName` to `imageNames`.
This more flexible definition allows future templates to display a
carrousel of images instead of a single image.

Depends on #2831.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonidero tonidero tonidero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NachoSoto @tonidero