-
-
Notifications
You must be signed in to change notification settings - Fork 194
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 Http Foundation to Known Core #3262
base: dev
Are you sure you want to change the base?
Conversation
WIP on #3029 |
This concludes minimum integration of httpFoundation , ill continue working on changing all our superglobals with ->request() |
What's holding this up? My only comment is that we could go all-in on Symfony and just use that instead of wrapping Request/Response? https://symfony.com/doc/current/setup.html That could replace a lot of the CLI and dev server/testing infra that is currently unmaintained... |
@lindner currently there needs to be little bit of work done on the current session class of know to replace $_SESSSION super globals that's the only thing that is holding back this pull request.
The last bit remaining is Removing all $_SESSION variables that needs a little bit of work as I need to integrate symfony session with current IDNO/Session . Also while doing this I am finding few small bugs/improper way of handling things which I need to patch as part of this transition. @lindner I agree ill remove the wrapper and directly use Symfony Request/Response , thanks for suggestion. While I am finishing up Session integration, I would love to know thoughts of @benwerd on this PR PS: I am a full time college student and work part time as freelance programmer , had a busy week did not get time to pach $_SESSSION yet, will finish this by tommrow most probably |
…nto http-foundation
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.
First pass, will delve deeper later today.
@@ -1,4 +1,4 @@ | |||
<link href="<?php echo \Idno\Core\Idno::site()->config()->getStaticURL() ?>css/<?php echo $this->getModifiedTS('css/known.min.css'); ?>/known.min.css" rel="stylesheet"> |
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 is removing cache-busting for this file.
} | ||
} | ||
// if (!defined('KNOWN_UNIT_TEST')) { | ||
// if (!empty($_SERVER['PHP_SELF'])) { |
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.
remove this instead of commenting please
Here's what I fixed or added:
Here's why I did it:
So that non can be compatible with non apache environment like Swoole etc , and also integrate SymphonyResponse compatible package (like required for ActivityPub Integration)
Checklist: (
[x]
to check/tick the boxes)