-
Notifications
You must be signed in to change notification settings - Fork 153
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
Added more consistency to the code #17
Conversation
Ninir
commented
Feb 4, 2013
- Fixed namespaced files imports
- Improved comments
// If not redirects | ||
// Set flag for response handle and generate closure | ||
// If not redirecting | ||
// Sets the flag for the response handle and generate closure |
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.
@suncat2000 Before merging, just want to know if you are talking about the response handler or if something is missing?
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.
This comment refers to the line:
$this->needModifyResponse = true;
Probably better to write it:
/**
* If not need redirecting
*/
// Sets the flag for the response handle
$this->needModifyResponse = true;
// Checking the need to modify the Response and set closure
if ($this->needTabletResponseModify()) {
$this->deviceView->setTabletView();
return;
}
or something like that.
Thanks for help and sorry for my English!
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.
Yep it could. When you say:
// Sets the flag for the response handle
do you mean:
// Sets the flag for the response handleR
?
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.
Yes )
It's my mistake
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.
@suncat2000 Done :) if you think something else should be changed, please ask :)
@suncat2000 In the DeviceView class, why not using strict comparison for |
I'm sorry I did not quite understand what you mean |
Well, for instance line 76 (following the link I gave above), you can see this: return ($this->viewType == self::VIEW_FULL); Why not using this (3 equals, comparing the type of vars): return $this->viewType === self::VIEW_FULL; |
I do not know why I was so wrote))) |
Added more consistency to the code
Creating a `new Cookie()` in SF4.2 triggers a deprecation notice, causing test suites to fail. The deprecation notice relates to the default value of two of the cookie constructor arguments. The notice suggests using the `Cookie::create()` method, but this is not available in Symfony versions older than 4.2. The alternative is to explicitly set values for all constructor arguments. This allows compatibility with older versions of Symfony while also supporting the newer versions in future. ``` The default value of the "$secure" and "$samesite" arguments of "Symfony\Component\HttpFoundation\Cookie::__construct"'s constructor will respectively change from "false" to "null" and from "null" to "lax" in Symfony 5.0, you should define their values explicitly or use "Cookie::create()" instead. Stack trace: #0 vendor/sentry/sentry/lib/Raven/ErrorHandler.php(127): Raven_Breadcrumbs_ErrorHandler->handleError(16384, 'The default val...', '/var/www/vendor...', 91, Array) suncat2000#1 [internal function]: Raven_ErrorHandler->handleError(16384, 'The default val...', '/var/www/vendor...', 91, Array) suncat2000#2 vendor/symfony/http-foundation/Cookie.php(91): trigger_error('The default val...', 16384) suncat2000#3 vendor/suncat/mobile-detect-bundle/SunCat/MobileDetectBundle/Helper/DeviceView.php(476): Symfony\Component\HttpFoundation\Cookie->__construct('device_view', 'full', Object(DateTime), '/', '', false, true) suncat2000#4 vendor/suncat/mobile-detect-bundle/SunCat/MobileDetectBundle/Helper/DeviceView.php(299): SunCat\MobileDetectBundle\Helper\DeviceView->createCookie('full') suncat2000#5 vendor/suncat/mobile-detect-bundle/SunCat/MobileDetectBundle/EventListener/RequestResponseListener.php(219): SunCat\MobileDetectBundle\Helper\DeviceView->modifyResponse('full', Object(Symfony\Component\HttpFoundation\Response)) suncat2000#6 vendor/suncat/mobile-detect-bundle/SunCat/MobileDetectBundle/EventListener/RequestResponseListener.php(177): SunCat\MobileDetectBundle\EventListener\RequestResponseListener->SunCat\MobileDetectBundle\EventListener\{closure}(Object(SunCat\MobileDetectBundle\Helper\DeviceView), Object(Symfony\Component\HttpKernel\Event\FilterResponseEvent)) suncat2000#7 vendor/symfony/event-dispatcher/EventDispatcher.php(212): SunCat\MobileDetectBundle\EventListener\RequestResponseListener->handleResponse(Object(Symfony\Component\HttpKernel\Event\FilterResponseEvent), 'kernel.response', Object(Symfony\Component\EventDispatcher\EventDispatcher)) suncat2000#8 vendor/symfony/event-dispatcher/EventDispatcher.php(44): Symfony\Component\EventDispatcher\EventDispatcher->doDispatch(Array, 'kernel.response', Object(Symfony\Component\HttpKernel\Event\FilterResponseEvent)) suncat2000#9 vendor/symfony/http-kernel/HttpKernel.php(189): Symfony\Component\EventDispatcher\EventDispatcher->dispatch('kernel.response', Object(Symfony\Component\HttpKernel\Event\FilterResponseEvent)) suncat2000#10 vendor/symfony/http-kernel/HttpKernel.php(171): Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object(Symfony\Component\HttpFoundation\Response), Object(Symfony\Component\HttpFoundation\Request), 1) suncat2000#11 vendor/symfony/http-kernel/HttpKernel.php(67): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) suncat2000#12 vendor/symfony/http-kernel/Kernel.php(198): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) suncat2000#13 vendor/symfony/http-kernel/Client.php(68): Symfony\Component\HttpKernel\Kernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) suncat2000#14 vendor/symfony/framework-bundle/Client.php(131): Symfony\Component\HttpKernel\Client->doRequest(Object(Symfony\Component\HttpFoundation\Request)) suncat2000#15 vendor/symfony/browser-kit/Client.php(405): Symfony\Bundle\FrameworkBundle\Client->doRequest(Object(Symfony\Component\HttpFoundation\Request)) suncat2000#16 tests/functional/Bundle/QuizBundle/Controller/PublicControllerTest.php(13): Symfony\Component\BrowserKit\Client->request('GET', 'http://localhos...') suncat2000#17 [internal function]: Weirdly\Bundle\QuizBundle\Controller\PublicControllerTest->testPublicQuiz() suncat2000#18 {main} Exited with code 1 ``` This pull request introduces those arguments and sets them explicitly, maintaining current behaviour with no backwards-breaking changes.