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

Enforcement of customizable type constraints for DependencyBuilder #93

Merged
merged 10 commits into from
Dec 7, 2023

Conversation

philippzagar
Copy link
Member

@philippzagar philippzagar commented Dec 6, 2023

Enforcement of customizable type constraints for DependencyBuilder

♻️ Current situation & Problem

As of now, the DependencyBuilder aggregates multiple types conforming to Module towards a DependencyCollection. However, the type constraint of the DependencyBuilder currently only enables the enforcement of the Module conformance, not the conformance to other, more specific types that may be useful for implementing a Domain Specific Language.

⚙️ Release Notes

  • Enable enforcement of customizable type constraints for DependencyBuilders via the newly introduced DependencyCollectionBuilder protocol

📚 Documentation

DocC docs added.

✅ Testing

--

📝 Code of Conduct & Contributing Guidelines

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

@philippzagar philippzagar added the enhancement New feature or request label Dec 6, 2023
@philippzagar philippzagar self-assigned this Dec 6, 2023
@philippzagar
Copy link
Member Author

philippzagar commented Dec 6, 2023

Thanks a lot to @Supereg for helping with the design/implementation!

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Merging #93 (641a774) into main (092eabc) will increase coverage by 0.08%.
The diff coverage is 53.85%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #93      +/-   ##
==========================================
+ Coverage   91.46%   91.53%   +0.08%     
==========================================
  Files          30       31       +1     
  Lines         655      661       +6     
==========================================
+ Hits          599      605       +6     
  Misses         56       56              
Files Coverage Δ
Sources/Spezi/Dependencies/DependencyBuilder.swift 100.00% <ø> (+52.18%) ⬆️
...rces/Spezi/Dependencies/DependencyCollection.swift 97.30% <100.00%> (+0.24%) ⬆️
...Spezi/Dependencies/DependencyPropertyWrapper.swift 90.75% <100.00%> (+0.55%) ⬆️
Sources/Spezi/Module/ModuleBuilder.swift 100.00% <ø> (ø)
...ezi/Dependencies/DependencyCollectionBuilder.swift 40.00% <40.00%> (ø)

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 092eabc...641a774. Read the comment docs.

Copy link
Member

@Supereg Supereg left a comment

Choose a reason for hiding this comment

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

Not sure if you already expect a review, but had a chance to look at the implementation and had a small comment. So I'm already submitting that now 👍

Looks great 🚀

Sources/Spezi/Dependencies/DependencyCollection.swift Outdated Show resolved Hide resolved
@philippzagar
Copy link
Member Author

Thanks a lot to @Supereg for helping with the design/implementation!

Thanks for the review, didn't expect it so quickly as I wanted to write some additional test cases :D

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 @philippzagar for the additions and thank you for @Supereg for the support! I added two smaller comments about docs but apart from this I am happy to see the PR being merged 👍

@philippzagar
Copy link
Member Author

@PSchmiedmayer Please merge the PR, small testing check is failing as the result builder default protocol implementation leads to a few patch coverage misses.

@PSchmiedmayer PSchmiedmayer merged commit d56ed07 into main Dec 7, 2023
7 of 8 checks passed
@PSchmiedmayer PSchmiedmayer deleted the feat/dependency-builder branch December 7, 2023 18:59
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants