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

App Info and App Store Version #116

Merged
merged 34 commits into from
Dec 3, 2020
Merged

App Info and App Store Version #116

merged 34 commits into from
Dec 3, 2020

Conversation

MortenGregersen
Copy link
Contributor

No description provided.

@MortenGregersen
Copy link
Contributor Author

I don't understand why the build fails. It doesn't look like the build has started on Bitrise.

Copy link
Owner

@AvdLee AvdLee left a comment

Choose a reason for hiding this comment

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

Thanks, this is great!

@SwiftLeeBot
Copy link
Collaborator

SwiftLeeBot commented Sep 1, 2020

Warnings
⚠️ Please provide a summary in the Pull Request description
⚠️

Sources/Models/AppStoreVersion.swift#L60 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L65 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L80 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L90 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L95 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L120 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L125 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L140 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L150 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L155 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L165 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L170 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L180 - Types should be nested at most 1 level deep (nesting)

⚠️

Sources/Models/AppStoreVersion.swift#L185 - Types should be nested at most 1 level deep (nesting)

⚠️

Tests/APIProviderTests.swift#L126 - cannot match several associated values at once, implicitly tupling the associated values and trying to match that instead

Messages
📖

View more details on Bitrise

📖 AppStoreConnect-Swift-SDK: Executed 160 tests, with 0 failures (0 unexpected) in 0.413 (1.043) seconds

AppStoreConnect_Swift_SDK.framework: Coverage: 81.18

File Coverage
ListAppInfosForApp.swift 97.22%
ReadAppInfoOfApp.swift 100.0%
ListAppStoreVersionsForApp.swift 94.29%
AppRelationship.swift 100.0%
AppStoreVersionRelationship.swift 0.0% ⚠️
AppInfoRelationship.swift 0.0% ⚠️
GetApp.swift 97.01%
ListApps.swift 91.21%

Generated by 🚫 Danger Swift against f554e3f

case .build:
self = try .build(Build(from: decoder))
default:
fatalError()
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • ⚠️ A fatalError call should have a message. (fatal_error_message)

@AvdLee
Copy link
Owner

AvdLee commented Sep 2, 2020

I don't understand why the build fails. It doesn't look like the build has started on Bitrise.

This is related to Bitrise which only starts the build after manual approval from my side.

Regarding the errors, do you have them after doing a clean build locally?

@MortenGregersen
Copy link
Contributor Author

MortenGregersen commented Sep 2, 2020

Regarding the errors, do you have them after doing a clean build locally?

It builds fine on my machine when I do a clean build in both Xcode 11.7 and Xcode 12 beta 6. In Xcode 12 I get 158 warnings saying "Immutable property will not be decoded because it is declared with an initial value which cannot be overwritten" for the let type property on all the models.

@cieslakdawid
Copy link
Contributor

@MortenGregersen I'm referring to AppStoreConnect-Swift-SDK.xcodeproj/project.pbxproj file.

For example the ListApp.swift is listed in source files and added to AppStoreConnect-Swift-SDK target. But as I don't see any changes that would add new files (e.g. AppInfo) to this target.

@MortenGregersen
Copy link
Contributor Author

MortenGregersen commented Oct 3, 2020

@MortenGregersen I'm referring to AppStoreConnect-Swift-SDK.xcodeproj/project.pbxproj file.

Got it. Didn't see the .xcodeproj in Githubs file list as it was shown as a folder... :) Maybe you are right.

@cieslakdawid
Copy link
Contributor

@MortenGregersen Great! Looks like an easy fix, but let me know if I can help :)

@MortenGregersen
Copy link
Contributor Author

I don't have the time to look at this right now. It would be awesome if you would try and add the files 👍

@cieslakdawid
Copy link
Contributor

@MortenGregersen Sure - I've added source files, but there are also some tests failing. I'll take care of them tomorrow and update

cieslakdawid and others added 3 commits October 4, 2020 13:17
- Sort files in the project file by the name
Add new files to the main target dependencies & update exected
@cieslakdawid
Copy link
Contributor

Hey @AvdLee, can we ask to rerun the CI on the latest commit?
Hopefully, some errors will go away now.

Repository owner deleted a comment from MortenGregersen Oct 4, 2020
Repository owner deleted a comment from AvdLee Oct 4, 2020
@cieslakdawid
Copy link
Contributor

Errors are gone, except the formatting one. Also looks like the coverage is quite low.
I might also help with that in upcoming days (probably Tue), unless you're a bit more available to finish up @MortenGregersen

@MortenGregersen
Copy link
Contributor Author

Errors are gone, except the formatting one. Also looks like the coverage is quite low.

I might also help with that in upcoming days (probably Tue), unless you're a bit more available to finish up @MortenGregersen

Fantastic! Thank you for the help.

I don't have the time to do any work on this the following weeks.

@github-actions
Copy link

github-actions bot commented Nov 4, 2020

This PR is stale because it has been open for 30 days with no activity. Remove the Stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Nov 4, 2020
* Update GetAppTests for the new filtering options

* Update ListAppsTests for the new filtering options

* Update tests for AppRelationship

* Add tests for generating the correct URL for ListAppStoreVersionsForApp endpoint

* Remove `limit` parameter from ReadAppInfoOfApp (cannot be applied for this endpoint)

* Fix the `appInfo` field key

* Fix the method signature to reflect better the appropriate resource id

* Add tests for ReadAppInfoOfAppTests endpoint

* Fix AppInfo key

* Add tests for ListapplnfosForApp for generating the correct URL

* Remove duplicated white space

* Comment out unimplemeted logic so the switch can be exhaustive
@MortenGregersen
Copy link
Contributor Author

@AvdLee I can't remove the "Stale" label. The CI should be run on this PR again after the latest commit.

@cieslakdawid
Copy link
Contributor

cieslakdawid commented Nov 29, 2020

We got a little bit more tests & hopefully the linter error fixed, so ideally, the CI turns green and we can close this one :)

@github-actions github-actions bot removed the Stale label Nov 30, 2020
public let data: AppStoreVersion.Relationships.AgeRatingDeclaration.Data?
public let links: AppStoreVersion.Relationships.AgeRatingDeclaration.Links

public struct Data: Codable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • ⚠️ Types should be nested at most 1 level deep (nesting)

@AvdLee
Copy link
Owner

AvdLee commented Dec 3, 2020

I think that we can ignore the nested types warnings here. I feel like it's useful in this case and helps to improve readability.

public let data: AppStoreVersion.Relationships.AppStoreVersionLocalizations.Data?
public let links: AppStoreVersion.Relationships.AppStoreVersionLocalizations.Links?

public struct Data: Codable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • ⚠️ Types should be nested at most 1 level deep (nesting)

@cieslakdawid
Copy link
Contributor

I agree about nested types. Also, the CI went green, so hopefully, we can merge now.
Thanks both 👏

@AvdLee
Copy link
Owner

AvdLee commented Dec 3, 2020

I had to merge in the base branch. Once CI succeeds again, I'll merge it in!

public let data: AppStoreVersion.Relationships.AppStoreVersionSubmission.Data?
public let links: AppStoreVersion.Relationships.AppStoreVersionSubmission.Links?

public struct Data: Codable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • ⚠️ Types should be nested at most 1 level deep (nesting)

public let type: String = "appStoreVersionLocalizations"
}

public struct Links: Codable {
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • ⚠️ Types should be nested at most 1 level deep (nesting)

@AvdLee AvdLee merged commit 99eabd8 into AvdLee:master Dec 3, 2020
@SwiftLeeBot
Copy link
Collaborator

Congratulations! 🎉 This was released as part of Release 1.2.0 🚀

Generated by GitBuddy

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.

4 participants