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

Change the TYPO3 AdditionalConfiguration.php template to extend the MAIL array - fixes #1042 #1043

Merged

Conversation

dmnkhhn
Copy link
Contributor

@dmnkhhn dmnkhhn commented Aug 10, 2018

The Problem/Issue/Bug:

Problem described in #1042

How this PR Solves The Problem:

The code in the template file is changed so that the array is extended instead of overwritten so that we have access to the full $GLOBALS['TYPO3_CONF_VARS']['MAIL'] array in TYPO3.

Manual Testing Instructions:

Testing this automatically was not possible as far as I know.

@CLAassistant
Copy link

CLAassistant commented Aug 10, 2018

CLA assistant check
All committers have signed the CLA.

@rfay rfay changed the title Change the TYPO3 AdditionalConfiguration.php template to extend the MAIL array - fixed #1042 Change the TYPO3 AdditionalConfiguration.php template to extend the MAIL array - fixes #1042 Aug 10, 2018
Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems totally reasonable if it tests out. I am in the dark about what gets lost this way, but still this is fine (after manual test).

Note that still the contents of AdditionalConfiguration.php can be changed on any project, just remove the #ddev-generated comment and ddev won't touch it any more.

@dmnkhhn
Copy link
Contributor Author

dmnkhhn commented Aug 10, 2018

There should be no negative impact as all other smtp settings are empty by default and they are not touched by this change.
There is only one MAIL setting that's not empty which is �transport_sendmail_command� and it is ignored because the transport is already set to smtp.

@rfay
Copy link
Member

rfay commented Aug 11, 2018

@dmnkhhn please do add the email you used for this commit to github per #1043 (comment) (And accept the CLA)- Thanks!

@dmnkhhn
Copy link
Contributor Author

dmnkhhn commented Aug 11, 2018

Done, thanks for the reminder!

@rfay rfay merged commit bce94d2 into ddev:master Aug 13, 2018
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.

None yet

3 participants