Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

When set -f option, no set -f option automatically #227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tzmfreedom
Copy link

@tzmfreedom tzmfreedom commented Jun 13, 2019

Zend\Mail\Transport\SendMail always append envelope-from parameter.
But I want to set any envelope-from.
In this PR, only when not set option -f, set -f option automatically.
This means if I set [email protected] or -bs [email protected], Zend\Mail\Transport\Sendmail doesn't set -f option.

@glensc
Copy link
Contributor

glensc commented Jun 18, 2019

without getting deeper to the actual changes, have some thoughts:

  1. seems weird to have a negative parameter. how about set positive parameter which is b default true, and can be set as false.
  2. maybe have a specific option (envelope related) rather generic "no override" as behavior could be different if want/want not to override some other parameter.

@tzmfreedom tzmfreedom force-pushed the only_use_set_parameter branch from 817654a to 92fe2c9 Compare June 19, 2019 12:16
@tzmfreedom
Copy link
Author

Thank you for you reply and advice !

I rethink, revise my code and changed this PR goal.

On this PR, only when not set option -f, set -f option automatically.
This means if I set [email protected] or -bs [email protected], Transport\Sendmail doesn't set -f option.
So there is no negative parameter, no additional method.

@glensc
Copy link
Contributor

glensc commented Jun 19, 2019

you should update PR title as well. and the body is a bit hard to read as well.

@tzmfreedom tzmfreedom changed the title add noOverrideParameters method not to override parameter When set -f option, no set -f option automatically Jun 20, 2019
@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-mail; a new issue has been opened at laminas/laminas-mail#17.

@weierophinney
Copy link
Member

This repository has been moved to laminas/laminas-mail. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

  • Squash all commits in your branch (git rebase -i origin/{branch})
  • Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
  • Run the laminas/laminas-migration tool on the code.
  • Clone laminas/laminas-mail to another directory.
  • Copy the files from the second bullet point to the clone of laminas/laminas-mail.
  • In your clone of laminas/laminas-mail, commit the files, push to your fork, and open the new PR.
    We will be providing tooling via laminas/laminas-migration soon to help automate the process.

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

Successfully merging this pull request may close these issues.

3 participants