Skip to content

Clean up ext/session errors - #6111

Closed
kocsismate wants to merge 3 commits into
php:masterfrom
kocsismate:session-callback
Closed

Clean up ext/session errors#6111
kocsismate wants to merge 3 commits into
php:masterfrom
kocsismate:session-callback

Conversation

@kocsismate

@kocsismatekocsismate commented Sep 10, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Apparently, ext/session has the most typos and grammatical errors in its error messages among all the extensions.

kocsismate
kocsismate commented Sep 10, 2020
View reviewed changes
Comment threadext/session/mod_files.c Outdated
Comment threadext/session/session.c Outdated
@kocsismatekocsismate mentioned this pull request Sep 10, 2020
Closed
71 tasks
nikic
nikic reviewed Sep 11, 2020
View reviewed changes
Comment threadext/session/mod_user.c Outdated
Comment threadext/session/session.c Outdated
Comment threadext/session/session.c Outdated
Comment threadext/session/session.c Outdated
@nikic

nikic commented Sep 11, 2020

Copy link
Copy Markdown
Member

Some of those error conditions can likely be promoted. But I guess it's hard to say which for this extension.

@kocsismate

kocsismate commented Sep 11, 2020

Copy link
Copy Markdown
MemberAuthor

@nikic Yeah, I didn't do much promotions exactly because of this reason :( I think the cases could be promoted when a session is not active. The other issues with session handling (e.g. headers are already sent or session is already active) seem too disruptive for a promotion.

kocsismate
kocsismate commented Sep 11, 2020
View reviewed changes
Comment threadext/session/tests/session_set_save_handler_type_error.phpt Outdated
@kocsismate
kocsismate deleted the session-callback branch September 11, 2020 10:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@nikicnikicnikic approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Warning promotion

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kocsismate@nikic