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

Allow to load multiple modules of the same type #110

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Jul 21, 2024

Allow to load multiple modules of the same type

♻️ Current situation & Problem

Currently, Spezi enforces that there is maximum one module of the same type loaded at a time. By allowing the Module system to used much more dynamically via #105, we found that certain types of Modules might exist multiple times in the system (e.g., a Bluetooth device type modeled as a Spezi Module might have two physical devices connected at the same time).
This PR makes the necessary infrastructure changes to support loading multiple modules of the same type. A check that the same module can only be loaded once is still in place.
Restructuring the @Dependency to support multiple modules of the same type is not trivial and will be addressed in a follow-up PR which is tracked in #111.

⚙️ Release Notes

  • Allow to load multiple modules of the same type.

📚 Documentation

--

✅ Testing

Additional unit testing was added to verify behavior.

📝 Code of Conduct & Contributing Guidelines

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

Copy link

codecov bot commented Jul 21, 2024

Codecov Report

Attention: Patch coverage is 93.83562% with 9 lines in your changes missing coverage. Please review.

Project coverage is 88.03%. Comparing base (fffb691) to head (485c60c).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #110      +/-   ##
==========================================
+ Coverage   85.85%   88.03%   +2.18%     
==========================================
  Files          45       47       +2     
  Lines        1392     1470      +78     
==========================================
+ Hits         1195     1294      +99     
+ Misses        197      176      -21     
Files Coverage Δ
Sources/Spezi/Dependencies/DependencyManager.swift 78.64% <100.00%> (+0.19%) ⬆️
...dencies/Property/DependencyCollectionBuilder.swift 40.00% <ø> (ø)
.../Dependencies/Property/DependencyDeclaration.swift 100.00% <ø> (ø)
...endencies/Property/DependencyPropertyWrapper.swift 92.69% <100.00%> (+3.40%) ⬆️
Sources/Spezi/Module/Module.swift 100.00% <ø> (ø)
Sources/Spezi/Utilities/DynamicReference.swift 100.00% <100.00%> (ø)
...pezi/Spezi/KnowledgeSources/StoredModulesKey.swift 97.78% <97.78%> (ø)
Sources/Spezi/Spezi/Spezi.swift 98.07% <95.92%> (+2.58%) ⬆️
...i/Dependencies/Property/DependencyCollection.swift 93.94% <62.50%> (+3.47%) ⬆️
...pezi/Dependencies/Property/DependencyContext.swift 92.71% <88.47%> (+8.84%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afdf500...485c60c. Read the comment docs.

@Supereg Supereg marked this pull request as ready for review July 22, 2024 11:27
@Supereg Supereg requested a review from PSchmiedmayer July 22, 2024 11:28
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Thank you for all the improvements here @Supereg; looks great!

@Supereg Supereg merged commit d87e3d8 into main Jul 23, 2024
15 checks passed
@Supereg Supereg deleted the feature/multi-module-loading branch July 23, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants