Skip to content

Update constructor_injection.py - #284

Closed
vliweihan wants to merge 1 commit into
faif:masterfrom
vliweihan:patch-1
Closed

Update constructor_injection.py#284
vliweihan wants to merge 1 commit into
faif:masterfrom
vliweihan:patch-1

Conversation

@vliweihan

Copy link
Copy Markdown

Hi master. The now() function should add @staticmethod, otherwise "current_time = self.time_provider.now()" will fail

The now() function should add @staticmethod, otherwise "current_time = self.time_provider.now()" will fail
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #284 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #284   +/-   ##
=======================================
  Coverage   72.99%   72.99%           
=======================================
  Files          62       62           
  Lines        2177     2177           
=======================================
  Hits         1589     1589           
  Misses        588      588
Impacted Files Coverage Δ
patterns/dft/constructor_injection.py 100% <100%> (ø) ⬆️

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 e0e9bfc...ece01ab. Read the comment docs.

@gyermolenko

gyermolenko commented Mar 1, 2019

Copy link
Copy Markdown
Contributor

I think that before this change we should probably clean-up dft/constructor_injection.py and tests/dft/test_constructor_injection.py.
I could probably do that today.

@faif

faif commented Mar 5, 2019

Copy link
Copy Markdown
Owner

The now() function is gone after merging #285 so no need to merge #284

@faif faif closed this Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants