8000 feat: Change session manipulation methods from `private` to `protected` by michalsn · Pull Request #1113 · codeigniter4/shield · GitHub
[go: up one dir, main page]
Skip to content

feat: Change session manipulation methods from private to protected#1113

Merged
kenjis merged 2 commits intocodeigniter4:developfrom
michalsn:session
Jun 8, 2024
Merged

feat: Change session manipulation methods from private to protected#1113
kenjis merged 2 commits intocodeigniter4:developfrom
michalsn:session

Conversation

@michalsn
Copy link
Member
@michalsn michalsn commented May 3, 2024

Description
This PR changes the session manipulation methods from private to protected, so we can use them if we extend the Authentication\Authenticators\Session class.

Reference: #1111

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

kenjis
kenjis reviewed May 6, 2024
View reviewed changes
kenjis
kenjis reviewed May 6, 2024
View reviewed changes
kenjis
kenjis reviewed May 6, 2024
View reviewed changes
@kenjis
Copy link
Member
kenjis commented May 6, 2024

This Session class is already very long, and would violate the SRP.
Don't we need a new class for these methods?

@michalsn
Copy link
Member Author
michalsn commented May 6, 2024

The methods related to the user session will give us about 50 lines of code. I don't see the point of moving them to a separate class - especially if they are to remain non-public.

@kenjis
Copy link
Member
kenjis commented May 13, 2024

The session data manipulation is needed only for the Session authenticator.
So extracting them into a class does not seem to help much in the future.
If we extract classes, we should extract another functions, but so far there is no need.

By the way, is this an enhancement or a bug fix?
In other words, should the next release be minor version up?

@michalsn
Copy link
Member Author
michalsn commented May 13, 2024

I would say this is an enhancement.

@michalsn michalsn changed the title Change session manipulation methods from private to protected feat: Change session manipulation methods from private to protected May 13, 2024
@kenjis kenjis added the enhancement New feature or request label May 13, 2024
@kenjis
Copy link
Member
kenjis commented May 13, 2024

Then it should be released as v1.1.0.

@michalsn
Copy link
Member Author
michalsn commented May 13, 2024

Okay. I'm not familiar with the Shield release cycle, so it's up to you when you'd like to merge this.

@kenjis kenjis merged commit aa40f9c into codeigniter4:develop Jun 8, 2024
@kenjis kenjis mentioned this pull request Jun 13, 2024
Merged
5 tasks
@michalsn michalsn deleted the session branch July 6, 2025 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kenjis kenjis kenjis approved these changes

@datamweb datamweb datamweb approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@michalsn @kenjis @datamweb
0