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

Support container interfaces for get() return type #39

Merged
merged 2 commits into from
Nov 25, 2022

Conversation

InvisibleSmiley
Copy link
Contributor

No description provided.

@Slamdunk
Copy link
Owner

Slamdunk commented Nov 8, 2022

I see that the build is not passing, can you have a look at it please?

@InvisibleSmiley
Copy link
Contributor Author

The reported errors do not seem to be caused by the changes I made; some of them might even be temporary problems that might go away when re-running the checks (which I do not have the permissions to trigger I think). The codesniffer changes are probably caused by the fact that the rules were changed meanwhile.

I'm pretty sure current master will fail the same. Not sure what your policy is who should fix those issues. In our libraries we use a composer.lock so dependencies (especially dev ones like codesniffer rules) cannot change without explicit action.

I would assume that a PR like mine that is targeted at fixing a certain issue should only do that.

So, how shall we proceed?

@Slamdunk
Copy link
Owner

Can you rebase this please?

@codecov-commenter
Copy link

Codecov Report

Base: 44.00% // Head: 43.16% // Decreases project coverage by -0.83% ⚠️

Coverage data is based on head (8038545) compared to base (5b1eabe).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #39      +/-   ##
============================================
- Coverage     44.00%   43.16%   -0.84%     
- Complexity      107      109       +2     
============================================
  Files            20       23       +3     
  Lines           275      278       +3     
============================================
- Hits            121      120       -1     
- Misses          154      158       +4     
Impacted Files Coverage Δ
...actServiceLocatorGetDynamicReturnTypeExtension.php 0.00% <0.00%> (ø)
.../InteropContainerGetDynamicReturnTypeExtension.php 0.00% <0.00%> (ø)
...inas/PsrContainerGetDynamicReturnTypeExtension.php 0.00% <0.00%> (ø)
...as/ServiceManagerGetDynamicReturnTypeExtension.php 0.00% <ø> (ø)
src/UnmappedAliasServiceLocatorProxy.php 45.45% <ø> (ø)
src/Type/Laminas/PluginMethodReflection.php 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@InvisibleSmiley
Copy link
Contributor Author

Rebased and "fixed" the coding standard issues.
Not sure what to do about the rest (Codecov) though.

@Slamdunk Slamdunk added the enhancement New feature or request label Nov 25, 2022
@Slamdunk Slamdunk merged commit f3fa53f into Slamdunk:master Nov 25, 2022
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.

4 participants