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

[scr 1.4] constructor for injection must be public #1218 Pull Request #1474

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

EverettHanke
Copy link

Ticket #1218 solution.

Gives a warning when a constructor is injectable but is not public/annotated with @activiate telling users to make constructor public.

Copy link

Test Results

   285 files  ±0     285 suites  ±0   53m 23s ⏱️ -15s
 3 586 tests ±0   3 510 ✅ ±0   76 💤 ±0  0 ❌ ±0 
10 950 runs  ±0  10 719 ✅ ±0  231 💤 ±0  0 ❌ ±0 

Results for commit 658fc71. ± Comparison against base commit ef694eb.

@HannesWell
Copy link
Member

Hi @EverettHanke thanks for this update.

First please squash all your commits into one with a meaningful message that covers the change as whole and force push it to the branch of this PR. Please don't create a new PR, you can simply update its change by using git force-push!

Furthermore please revert all brace re-formatting.
And as far as I can tell the requested changes from #1464 (review) are not address, are they?

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.

2 participants