Skip to content
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

Flash and segmentation features added #24

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

rotexdegba
Copy link

  • Moved exception methods into the new trait \Josantonius\Session\ExceptionThrowingMethodsTrait
  • Added a new Exception (\Josantonius\Session\Exceptions\EmptySegmentNameException) used in the new Session Segment class (\Josantonius\Session\FlashableSessionSegment)
  • Added a new Session Segment Class (\Josantonius\Session\FlashableSessionSegment) that stores data in a sub-array inside $_SESSION & provides data flashing functionality
  • Added a new Interface (\Josantonius\Session\SessionInterface)
  • Josantonius\Session\Session changes:
    • Now implements \Josantonius\Session\SessionInterface
    • start method now sets the session name before starting the session if the name option is specified
    • Exception throwing method have been moved to \Josantonius\Session\ExceptionThrowingMethodsTrait
    • Uses \Josantonius\Session\ExceptionThrowingMethodsTrait to make exception throwing methods available
  • Added tests to test all features in \Josantonius\Session\FlashableSessionSegment
  • Updated README.md with instructions on how to use \Josantonius\Session\FlashableSessionSegment

@rotexdegba
Copy link
Author

Please let me know if you have any questions about this pull request.
Thanks for all the good work you've done in creating and maintaining this library so far.

@josantonius
Copy link
Owner

Thank you for the PR!

I’ll review it thoroughly when I have a bit more time, as there’s quite a lot to go through.

@rotexdegba
Copy link
Author

Thank you for the PR!

I’ll review it thoroughly when I have a bit more time, as there’s quite a lot to go through.

You are welcome. Let me know if you need explanation on anything in the pull request. I realize that the documentation I added was a bit wordy, maybe we could tighten it up after you merge the pull request.

Thanks

@josantonius
Copy link
Owner

Thank you very much for your contribution to the Session library and for the effort you've put into enhancing its functionality. After reviewing the changes, I have come to the conclusion that the primary goal of this library is to encapsulate session handling in an object-oriented way. While the FlashableSessionSegment class offers valuable features, it seems to extend the library beyond its intended scope.

The other proposed changes, which are not related to FlashableSessionSegment, look good, and I’d be happy to incorporate them into the library. I’ll keep this PR open in case you'd like to modify it to include only those non-flash-related updates.

If you decide to create a separate library for the FlashableSessionSegment functionality, I’d be glad to support it by adding a link to your library in my README. This way, users who are interested in that specific feature can benefit from your work, while we keep this library focused on its main purpose.

Again, thank you for your valuable contribution. I appreciate your understanding and look forward to possibly collaborating in the future.

@rotexdegba
Copy link
Author

Thank you very much for your contribution to the Session library and for the effort you've put into enhancing its functionality. After reviewing the changes, I have come to the conclusion that the primary goal of this library is to encapsulate session handling in an object-oriented way. While the FlashableSessionSegment class offers valuable features, it seems to extend the library beyond its intended scope.

The other proposed changes, which are not related to FlashableSessionSegment, look good, and I’d be happy to incorporate them into the library. I’ll keep this PR open in case you'd like to modify it to include only those non-flash-related updates.

If you decide to create a separate library for the FlashableSessionSegment functionality, I’d be glad to support it by adding a link to your library in my README. This way, users who are interested in that specific feature can benefit from your work, while we keep this library focused on its main purpose.

Again, thank you for your valuable contribution. I appreciate your understanding and look forward to possibly collaborating in the future.

Thanks for the reply. I will create a fork of this project that contains the FlashableSessionSegment functionality. I will find time to create another PR with the changes not related to FlashableSessionSegment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants