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

Fixed method that removed part from parts in Mime\Message #3638

Closed

Conversation

RWOverdijk
Copy link
Contributor

Now using current() to get the single mime part so we don't unset it for future use.

@Maks3w
Copy link
Member

Maks3w commented Jan 31, 2013

Please add a test case

@RWOverdijk
Copy link
Contributor Author

How do you want me to test that it's working? It's now not removing parts. Do you want me to write a check to see if parts were removed?

@Maks3w
Copy link
Member

Maks3w commented Feb 1, 2013

Yes, because without test the risk of to have a regression is high. I.e someone do some PR for optimization and opts for change your current() again

@weierophinney
Copy link
Member

@RWOverdijk I agree with @Maks3w -- incremental improvements need tests so we can ensure we don't introduce regressions later. I realize that the MIME component is not well-tested -- but with improvements like this, we can add tests, and make it more solid going forward.

And many thanks in advance!

@RWOverdijk
Copy link
Contributor Author

I'll add a test when I find a small bit of time. Currently I have no time.

On 5 feb. 2013, at 17:33, weierophinney notifications@github.com wrote:

@RWOverdijk I agree with @Maks3w -- incremental improvements need tests so we can ensure we don't introduce regressions later. I realize that the MIME component is not well-tested -- but with improvements like this, we can add tests, and make it more solid going forward.

And many thanks in advance!


Reply to this email directly or view it on GitHub.

weierophinney added a commit that referenced this pull request Feb 5, 2013
- Wrote unit test that failed before changes, passes afterwards
weierophinney added a commit that referenced this pull request Feb 5, 2013
@ghost ghost assigned weierophinney Feb 5, 2013
@weierophinney
Copy link
Member

I've written a test and included it on merge.

@RWOverdijk
Copy link
Contributor Author

Thanks. Sorry for not adding the test. I'll add it next time :)

weierophinney added a commit to zendframework/zend-mime that referenced this pull request May 15, 2015
- Wrote unit test that failed before changes, passes afterwards
weierophinney added a commit to zendframework/zend-mime that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-mime that referenced this pull request May 15, 2015
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