-
Notifications
You must be signed in to change notification settings - Fork 106
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 NS to demos #1200
Add NS to demos #1200
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1200 +/- ##
=============================================
- Coverage 72.18% 70.61% -1.58%
Complexity 2559 2559
=============================================
Files 130 130
Lines 6271 6271
=============================================
- Hits 4527 4428 -99
- Misses 1744 1843 +99
Continue to review full report at Codecov.
|
@ibelar, hi, you were refactoring these week or two ago... Can you check the changes and resolve the class names with the current CS settings? I think you there are only two options
Then this PR is complete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mvorisek please fix conflicts and then it's ready to merge.
Not everything was finished here, but let's merge this in and if needed create new PR with additional changes in demo scripts. |
@mvorisek - this did make demos harder to read. Their purpose was to teach others how to use features - but with anonymous classes it is a non-standard approach. another consideration could be to assign dynamic classes into objects (drop get_class). |
@romaninsh this is needed for CS to work - the only option is to separate the classes with proper autoloading. |
fix #1199
Needed even if there is no autoloading on these, CS fixer otherwise behave differently without NS (and PHP too, as leading
\
is not required otherwise for absolute names)