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

Add "error_log_mode" setting #7901

Closed
wants to merge 2 commits into from
Closed

Conversation

mikhainin
Copy link
Contributor

We created this patch some while ago (back in 2011 :D) and decided that it is probably worth sharing with the community. Sometimes we might want to set other permissions for the created logfile (e.g. 0664) but umask only allows us to reduce the permissions, not to increase.

I would appreciate it if you give your opinion and tell me if something might be changed

@bukka
Copy link
Member

bukka commented Jan 17, 2022

I think it makes sense. Would be good to add a test for this though. Should be quite simple. Something like setting this INIs with location in the test dir, log error and checking perms using fileperms...

@mikhainin
Copy link
Contributor Author

Thanks for your comment - I'll add tests

@mikhainin
Copy link
Contributor Author

ouch, I think I did a mess here. Shall I re-create pull-request?
Or there is a better way?

@Girgias
Copy link
Member

Girgias commented Jan 28, 2022

ouch, I think I did a mess here. Shall I re-create pull-request? Or there is a better way?

You should rebase on top of master instead of merging master into your branch.

@mikhainin
Copy link
Contributor Author

ouch, I think I did a mess here. Shall I re-create pull-request? Or there is a better way?

You should rebase on top of master instead of merging master into your branch.

I was sure that was exactly what I did :/

@Girgias
Copy link
Member

Girgias commented Jan 28, 2022

You should rebase on top of master instead of merging master into your branch.

I was sure that was exactly what I did :/

Doesn't seem like try:

git checkout master
git pull
git rebase master error-log-mode
git push -f

@mikhainin
Copy link
Contributor Author

Oh, much better. Thank you so much!

@mikhainin
Copy link
Contributor Author

We can probably remove the "Waiting on Author" label, I've added a test and am not sure what I can do more. Open for ideas/requirements, though

@ramsey ramsey added the Feature label May 27, 2022
@ramsey ramsey added this to the PHP 8.2 milestone May 27, 2022
@bukka
Copy link
Member

bukka commented Jul 18, 2022

Merged in ffdf25a . Thanks!

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

Successfully merging this pull request may close these issues.

None yet

5 participants