Remove Config\App session items #7000
Conversation
kenjis
added
bug
|
|
||
| // DEPRECATED COOKIE MANAGEMENT | ||
| $this->cookiePath = $config->cookiePath ?? $this->cookiePath; | ||
| $this->cookieDomain = $config->cookieDomain ?? $this->cookieDomain; | ||
| $this->cookieSecure = $config->cookieSecure ?? $this->cookieSecure; | ||
| $this->cookieSameSite = $config->cookieSameSite ?? $this->cookieSameSite; | ||
|
|
||
| /** @var CookieConfig $cookie */ | ||
| /** @var CookieConfig|null $cookie */ |
There was a problem hiding this comment.
It seems we can't remove the Session's dependency on the Config\App without first removing the cookie items in Config\App and making the Config\Cookie mandatory...
There was a problem hiding this comment.
If everyone would be okay with removing session items from Config\App in 4.4, then I see no reason why we can't do the same with cookies.
There was a problem hiding this comment.
I sent a PR #7011 to remove cookie items.
Would everyone be okay with removing items from Config\App in 4.4?
kenjis
force-pushed
the
remove-config-app-session
branch
5 times, most recently
from
7e63407
to
83cbf59
Compare
kenjis
force-pushed
the
remove-config-app-session
branch
from
83cbf59
to
d58f098
Compare
| @@ -105,26 +105,19 @@ abstract class BaseHandler implements SessionHandlerInterface | |||
| */ | |||
| protected $ipAddress; | |||
|
|
|||
| public function __construct(AppConfig $config, string $ipAddress) | |||
| public function __construct(SessionConfig $session, string $ipAddress) | |||
There was a problem hiding this comment.
Yes and no. Almost no one writes their own session handlers, so in this particular case, the impact would be minimal. Although, I haven't thought of any other possible problems yet.
|
Sooner or later, v5 will come. It is more a question of where we want to draw the line. Will clearing the deprecated code and fixing some "bugs" will be enough to release v5, or are we hoping for some new features and major code rewrites? Personally, I think that v5 doesn't have to be followed by some spectacular breakthrough. Fixing a few things and cleaning up the codebase would be enough. |
|
@michalsn I don't disagree, but keep in mind that ultimately the major release cycle is governess by the CodeIgniter Foundation. While it's in a bit of a limbo, the last deliberation on the subject yielded a strong preference for keeping major releases to a minimum. We've been able to accomplish a lot in v4 since it's release, but at the cost of lots of breaking changes. I don't want to take v5 lightly, because I don't want to be stuck bearing these same inconsistencies and tension between proper implementation and breaking releases. I do think a focus on refactoring over new features makes sense for v5, since v4 and it's accompanying repos was so feature-focused. |
|
@MGatner Of course, deciding when something will be released is not my role. We have already deprecated many things in the codebase without giving any date for removing them. So common sense suggests that they will be removed in the next major version - not before. That's my only concern here. I'm not against refactoring, but it would be nice to have some path/policy for removing deprecated features so that developers won’t be surprised that some feature is gone in the next minor release. Even announcing that deprecated functionality may disappear anytime would be okay. |
I sent a PR #7021 |
Needs #6998Needs #7011
Description
Session should not depends on
Config\App. See #4297 (comment)Config\Appsession itemsConfig\SessioninSessionChecklist: