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

Remove dynamic_form? #1110

Closed
mnquintana opened this issue Jan 26, 2015 · 8 comments · Fixed by #1113
Closed

Remove dynamic_form? #1110

mnquintana opened this issue Jan 26, 2015 · 8 comments · Fixed by #1113

Comments

@mnquintana
Copy link
Contributor

So, looking at our Gemfile, I noticed that we're using dynamic_form, but I can't figure out why. It doesn't do anything that simple_form can't already do (and in fact does a whole lot less), and it's unmaintained. Was this to fix some form view errors when we upgraded to Rails 4? Either way, it should be replaced completely with simple_form, which we already use.

@orenyk
Copy link
Contributor

orenyk commented Jan 26, 2015

@caseywatts, I blame you :-P (e27a92f)

I have no idea if we're using it or why, but this really highlights the need for #600.

@jasonkliu
Copy link

So, uh, it used to be a plugin in Shifts and in the process of upgrading from Rails 2 to Rails 3, I removed it and readded it as a gem (YaleSTC/shifts@c701d82). From my comments, it seems to be needed for f.error_messages.

@jasonkliu
Copy link

More info here: YaleSTC/shifts#279

@mnquintana
Copy link
Contributor Author

Ah okay. Looking into it a bit more, we don't actually need f.error_messages at all, as long as we're using simple_form (and we should be), since it handles errors by default (and inline, instead of in a long list at the top).

@jasonkliu
Copy link

Haha YaleSTC/shifts#130 🌜

@mnquintana
Copy link
Contributor Author

@jasonkliu Wait what about YaleSTC/shifts#130?

@jasonkliu
Copy link

Dynamic form can be easily removed -- a long time ago, we just tried to put plugins and gems in to make them work. (It should be removed from Shifts too)

@orenyk
Copy link
Contributor

orenyk commented Jan 28, 2015

As far as I can tell it's not doing anything; we only use it on the new reservation / request form and the only thing we're validating there is the presence of a justification for a request and it appears as though that's independent of the errors we're displaying. I think we can safely remove dynamic_form and those two lines, and when we get around to #237 we'll be refactoring the view anyway. Experimentally, removing the gem and the two lines (here and here) broke nothing (tested those two forms and ran specs). I'll open a simple PR but I think this should be fine.

@orenyk orenyk added this to the 5.1.0 milestone Jan 28, 2015
@orenyk orenyk self-assigned this Jan 28, 2015
orenyk added a commit that referenced this issue Feb 17, 2015
@orenyk orenyk mentioned this issue Feb 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants