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

2.2.0RC1 Form\View\Helper\FormRow "partial view" messed up #4405

Closed
Slamdunk opened this issue May 3, 2013 · 8 comments
Closed

2.2.0RC1 Form\View\Helper\FormRow "partial view" messed up #4405

Slamdunk opened this issue May 3, 2013 · 8 comments
Milestone

Comments

@Slamdunk
Copy link
Contributor

Slamdunk commented May 3, 2013

Something went wrong during some merge, and now these two feature

https://github.com/zendframework/zf2/blob/release-2.2.0rc1/library/Zend/Form/View/Helper/FormRow.php#L70-L78

Are orphans.

This commit introduced the new functionality ( without tests 😞 ) : 744cab1
And this one messed up stuffs during the merge: ee4e4fc

@weierophinney
Copy link
Member

Can you provide a PR to fix this? I'm having issues trying to understand what needs to change...

@Slamdunk
Copy link
Contributor Author

Slamdunk commented May 3, 2013

Me too 😄 I just found it while reading the source.
The main issue is that setPartial method was removed, so $partial is useless.

Btw, I'll try to understand what committer intended to do, and make a PR with tests to fix it...

@davidwindell
Copy link
Contributor

Yeesh, how on earth did my partial stuff for FormRow end up in the repo O_o.

@weierophinney
Copy link
Member

@davidwindell Does that mean #4412 is unnecessary, or that different changes need to be made, or ...?

@Slamdunk
Copy link
Contributor Author

Slamdunk commented May 3, 2013

@weierophinney I don't know what @davidwindell meant to code, but I found the bug because I was looking exactly at that feature 😄

@davidwindell
Copy link
Contributor

@weierophinney I'm happy for it to stay in, I need to review #4412 to see if it meets our requirements.

I had planned to PR the partial support in the future and didn't realise it had been merged.

@weierophinney
Copy link
Member

@davidwindell Drop a note in #4412 when you've reviewed it, and let me know if/when it is ready.

Thanks!

@davidwindell
Copy link
Contributor

@weierophinney will do ASAP

weierophinney added a commit that referenced this issue May 6, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants