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

Add public constructor to internal classes which are instantiated via IoC #19

Merged

Conversation

marco-bertschi
Copy link
Contributor

Fixes #17

@codecov
Copy link

codecov bot commented Nov 12, 2019

Codecov Report

Merging #19 into release/1.0.0 will increase coverage by 0.17%.
The diff coverage is 57.69%.

Impacted file tree graph

@@                Coverage Diff                @@
##           release/1.0.0      #19      +/-   ##
=================================================
+ Coverage          45.09%   45.27%   +0.17%     
=================================================
  Files                100      100              
  Lines               1785     1831      +46     
  Branches             117      117              
=================================================
+ Hits                 805      829      +24     
- Misses               954      976      +22     
  Partials              26       26
Impacted Files Coverage Δ
...mpare/Helper/EqualityComparerHelperRegistration.cs 100% <ø> (ø) ⬆️
...eTools/Convert/Strategy/ConvertStrategyProvider.cs 0% <0%> (ø) ⬆️
...tructureTools/Convert/Value/ValueConvertMapping.cs 0% <0%> (ø) ⬆️
...ctureTools/Convert/Strategy/OperationCopySource.cs 0% <0%> (ø) ⬆️
...tureTools/Convert/Strategy/OperationCreateToOne.cs 0% <0%> (ø) ⬆️
...ureTools/Convert/Value/ConvertValueRegistration.cs 0% <0%> (ø) ⬆️
...rt/Strategy/OperationCopyValueIfTargetIsDefault.cs 0% <0%> (ø) ⬆️
...ert/Strategy/OperationCopyValueWithSourceFilter.cs 0% <0%> (ø) ⬆️
...t/Strategy/OperationCopyValueIfSourceNotDefault.cs 0% <0%> (ø) ⬆️
...nvert/Strategy/OperationCopyValueWithUpperLimit.cs 0% <0%> (ø) ⬆️
... and 20 more

Continue to review full report at Codecov.

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

Copy link
Collaborator

@pascalberger pascalberger left a comment

Choose a reason for hiding this comment

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

Would it make sense to also add a test case using Castle.Windsor?

@marco-bertschi
Copy link
Contributor Author

Would it make sense to also add a test case using Castle.Windsor?

Definitely, yes - I've create #22 for this.

@pascalberger
Copy link
Collaborator

Please rebase on release/1.0.0 branch and set target branch to release/1.0.0 on this pull request.

@marco-bertschi marco-bertschi force-pushed the topic/marco-bertschi/public_constructor branch 2 times, most recently from 7eeecb5 to cebf232 Compare November 15, 2019 13:47
@marco-bertschi marco-bertschi changed the base branch from develop to release/1.0.0 November 15, 2019 13:48
@marco-bertschi marco-bertschi force-pushed the topic/marco-bertschi/public_constructor branch from cebf232 to 46e59fc Compare November 15, 2019 13:50
@marco-bertschi
Copy link
Contributor Author

Please rebase on release/1.0.0 branch and set target branch to release/1.0.0 on this pull request.

Done

Copy link
Collaborator

@pascalberger pascalberger left a comment

Choose a reason for hiding this comment

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

Can we add a comment to the empty constructors why they are required? Otherwise someone will quite likely remove them in the future. Otherwise LGTM

@marco-bertschi marco-bertschi force-pushed the topic/marco-bertschi/public_constructor branch from 46e59fc to 4503d6c Compare November 16, 2019 15:11
@marco-bertschi
Copy link
Contributor Author

Can we add a comment to the empty constructors why they are required? Otherwise someone will quite likely remove them in the future. Otherwise LGTM

Done that.

…al with public constructor), where needed (#17)
@marco-bertschi marco-bertschi force-pushed the topic/marco-bertschi/public_constructor branch from 4503d6c to 04227d6 Compare November 16, 2019 15:14
@marco-bertschi marco-bertschi merged commit 574dcbb into release/1.0.0 Nov 16, 2019
@marco-bertschi marco-bertschi deleted the topic/marco-bertschi/public_constructor branch November 16, 2019 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants