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

doc typo: fix config key mismatch #36

Closed
wants to merge 1 commit into from

Conversation

pine3ree
Copy link

@pine3ree pine3ree commented Feb 9, 2021

Later in the example the referred configuration key is session instead of session_manager.

There is actually a supported session_manager configuration key, but the example stands apart and examines this custom configuration by itself. Also, after spotting a storage key under session_manager, developers may be led to believe that a storage key is supported by internal factories (...at least...I know I was).

kind regards

@froschdesign
Copy link
Member

froschdesign commented Feb 10, 2021

@pine3ree

Later in the example the referred configuration key is session instead of session_manager.

The second example works with a custom configuration structure. The factory for the session manager uses the config key:

if (isset($configService['session_manager'])

But here it is mixed and confusing. The entire page needs an update. Please see #7

@froschdesign froschdesign added the Invalid This doesn't seem right label Feb 10, 2021
@froschdesign
Copy link
Member

I have an update planned for this week: https://github.com/laminas/laminas-session/milestone/3

@pine3ree
Copy link
Author

ok @froschdesign, good! :-)

@pine3ree pine3ree deleted the patch-1 branch February 10, 2021 10:05
@froschdesign
Copy link
Member

@pine3ree
Your correction is fine, but the whole page is ugly! We need a useful description which helps in the real world and not a copy-paste solution that ignores the factory.

@pine3ree
Copy link
Author

Hello, @froschdesign. Yes, but I am sorry that I missed that old opened issue of the same nature (outdated docs).

@froschdesign
Copy link
Member

@pine3ree
Don't worry about it.

We have some projects for the documentation:

If you have some ideas, please open an issue report in the related issue tracker of the component and we will add it to the projects. Thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants