StanfordSpezi / SpeziTemplateApplication

Template application demonstrating the usage of the Stanford Spezi framework.
https://stanfordspezi.github.io/SpeziTemplateApplication
MIT License
97 stars 19 forks source link

implement sheet listing all package dependencies from SPM; #36

Closed NikolaiMadlener closed 10 months ago

NikolaiMadlener commented 12 months ago

Add an Open-Source Contributions Screen

:recycle: Current situation & Problem

The application currently does not provide any way to see all open-source tools used within the app, nor does it give access to all licenses used. See: https://github.com/StanfordSpezi/SpeziTemplateApplication/issues/29

:gear: Release Notes

Here is how it looks like: Simulator Screenshot - iPhone 14 Pro - 2023-09-18 at 15 42 39 Simulator Screenshot - iPhone 14 Pro - 2023-09-18 at 15 43 13

:books: Documentation

TBA

:white_check_mark: Testing

:pencil: Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

NikolaiMadlener commented 12 months ago

Just a first shot for my interpretation of #29.

PSchmiedmayer commented 12 months ago

Hi @NikolaiMadlener great to see you here! Let us know once you deem the PR ready for review and tag me if you have any questions! 👍

NikolaiMadlener commented 11 months ago

@PSchmiedmayer from my side this one is ready for review 🙂

PSchmiedmayer commented 11 months ago

Thank you @NikolaiMadlener! I will take a look at the PR later this week.

I have also approved it to run on our build infrastructure. It would be great if you can inspect the output of the builds and the resulting test coverage and see if you can add a small UI test to test that the rough functionality is working. No need to check all the dependencies in the UI itself. Please also take a look at the failing SwiftLint rule.

Regarding the build error: I suspect that we need to add the flag described here in our Fastlane Xcodebuild setup to ensure that the package can be run in the CI environment: https://forums.swift.org/t/telling-xcode-14-beta-4-to-trust-build-tool-plugins-programatically/59305

philippzagar commented 11 months ago

@NikolaiMadlener Nice stuff, love the dynamic fetching of packages via the PackageHelper! 🚀

codecov[bot] commented 11 months ago

Codecov Report

Merging #36 (c9b93db) into main (5f4e95c) will increase coverage by 0.71%. The diff coverage is 86.21%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36/graphs/tree.svg?width=650&height=150&src=pr&token=6EPKiz2l15&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi)](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) ```diff @@ Coverage Diff @@ ## main #36 +/- ## ========================================== + Coverage 81.79% 82.49% +0.71% ========================================== Files 32 36 +4 Lines 873 988 +115 ========================================== + Hits 714 815 +101 - Misses 159 173 +14 ``` | [Files](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) | Coverage Δ | | |---|---|---| | [TemplateApplication/Account/AccountSheet.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-VGVtcGxhdGVBcHBsaWNhdGlvbi9BY2NvdW50L0FjY291bnRTaGVldC5zd2lmdA==) | `81.82% <100.00%> (+2.88%)` | :arrow_up: | | [...eApplication/Contributions/ContributionsList.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-VGVtcGxhdGVBcHBsaWNhdGlvbi9Db250cmlidXRpb25zL0NvbnRyaWJ1dGlvbnNMaXN0LnN3aWZ0) | `100.00% <100.00%> (ø)` | | | [...plateApplication/Contributions/PackageHelper.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-VGVtcGxhdGVBcHBsaWNhdGlvbi9Db250cmlidXRpb25zL1BhY2thZ2VIZWxwZXIuc3dpZnQ=) | `63.64% <63.64%> (ø)` | | | [...emplateApplication/Contributions/PackageCell.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-VGVtcGxhdGVBcHBsaWNhdGlvbi9Db250cmlidXRpb25zL1BhY2thZ2VDZWxsLnN3aWZ0) | `86.49% <86.49%> (ø)` | | | [...pplication/Contributions/Package+LicenseType.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-VGVtcGxhdGVBcHBsaWNhdGlvbi9Db250cmlidXRpb25zL1BhY2thZ2UrTGljZW5zZVR5cGUuc3dpZnQ=) | `83.73% <83.73%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). Last update [5f4e95c...c9b93db](https://app.codecov.io/gh/StanfordSpezi/SpeziTemplateApplication/pull/36?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi).
NikolaiMadlener commented 11 months ago

Thanks for the great first feedback @PSchmiedmayer ! Will adopt all of the points. Exactly, ZStack will be removed once my PR with the addition to SpeziAccount is merged.

PSchmiedmayer commented 11 months ago

Sounds great; thank you @NikolaiMadlener 👍

NikolaiMadlener commented 11 months ago

@NikolaiMadlener Thank you for the great improvements and additions to the template application! I really like to approach using SwiftPackageList, which is a very smooth integration in the build pipeline.

Also, it's great to see the addition to SpeziAccount that will then embed this information in the Account overview; this should then replace the ZStack that you have in place here.

Before doing a more detailed review when these elements are in place, I can already suggest the following:

  1. The current implementation is based on a lot of the foundational work of the SwiftPackageList package and the views and examples from this package. I would suggest that you clearly highlight the code that you have reused from the project in the license identifier and properly provide attribution. Also, make sure that you are allowed to reuse the code here according to the license of the package.
  2. The current implementation does not use the Xcode/Swift localization mechanism. I have recently improved the setup in Use String Catalogues & Update to the Latest Spezi Packages #39, and this might directly benefit this PR. I would suggest that we localize all the values here as well.
  3. I would suggest merging the latest version of the main branch into your fork + feature branch to ensure that the latest version of the CI is used and that you can automatically check the builds in this PR.
  4. It would be great if we could support all the different license types that we currently have in our dependencies to complete the view.

And not sure if this is a misinterpretation, bit the version numbers seem to be slightly indented compared to the name: Screenshot 2023-10-04 at 1 30 20 AM

@PSchmiedmayer Your initial feedback should be implemented now. Regarding 1., SwiftPackageList is licensed under MIT so the usage should be fine. I also attributed the code that is based on the work of the SwiftPackageList repo (as a comment within the Package+LicenseType.swift).

Please let me know if there is anything else I can improve before this can be merged.