search

bootstrap-ruby / bootstrap_form

Official repository of the bootstrap_form gem, a Rails form builder that makes it super easy to create beautiful-looking forms using Bootstrap 5.
MIT License
1.64k stars 352 forks source link

Release bootstrap_form 4.0.0.alpha1 #361

Closed mattbrictson closed 6 years ago

mattbrictson commented 6 years ago

Goals

bootstrap_form has been "stuck" for several months with no major releases despite the looming release of Bootstrap v4 and new versions of Ruby and Rails. My goal as the de-facto maintainer of this project, and with the help of the community, is to get things moving again in 2018 and do a major new release.

Importantly, I also want to make this project more accessible to new contributors and easier to maintain. That means slimming down the version of Rails and Ruby we support to remove end-of-life (EOL) versions.

Background

Part of the reason we're stuck is that Bootstrap v4 has had an extremely long pre-release cycle (the first alpha shipped over two years ago). This left bootstrap_form in a kind of limbo: when and how do we fully commit to releasing a version of bootstrap_form that supports v4? Do we continue supporting v3 in the meantime? How best to divide our attention? Do we have to actively maintain two gems?

Into this mix add the fact that both Rails and Ruby are moving targets themselves. In particular, Rails 5.1 has brought the new form_with DSL, and the Rails team has strongly hinted that form_for, which is the basis for bootstrap_form, will eventually be deprecated.

Finally, the de-facto maintainer (yours truly) has been largely absent from the discussions surrounding these issues due to other time commitments. 😓

I feel strongly that bootstrap_form is a high-quality gem and useful to a large number of people, and I want to see us overcome these temporary setbacks.

Roadmap

The existing version of bootstrap_form, 2.7.0, is stable and works well for Bootstrap v3. I think it is time to end active development for 2.7.x and Bootstrap v3 support and go all-in on Bootstrap v4. Here's how the transition will work:

We will keep the version 2.7.0 of the gem as-is, and create a legacy-2.7 branch to allow for future patches but make it clear that it will not be under active development. We'll tag all existing issues and pull requests that aren't Bootstrap v4-related as "legacy" (and perhaps close them at some point if they aren't show-stopping bugs). Version 2.7.0 would be the last version of the gem to support Bootstrap v3.

Next, we make an effort to start merging all the Bootstrap v4 stuff into master. This will be a challenge since there are a lot of PRs pending, but if we can divide up the work among many contributors we can get through it.

The master branch will become bootstrap_form 4.0.0 (i.e skip 3.x entirely and go to version 4 to match Bootstrap v4). Version 4.0.0 will completely drop support for Bootstrap v3. While we are at it, to make future maintenance easier, we'll also drop support Ruby < 2.2 (these are EOL) as well as drop support for Rails < 5.0.

The master is where we will add support for form_with. This won't get back-ported to the legacy branch.

Once the master branch has settled, I'll release a 4.0.0.alpha1 to rubygems allow people to more easily test it out.

Our Progress

Also refer to the 4.0.0 milestone on GitHub.

How You Can Help

If you are using bootstrap_form and have questions about the roadmap or suggestions, I would like to hear from you! Please add a comment to this issue below.

PRs are welcome! If you would like to contribute code or documentation to help achieve our goals for bootstrap-form 4.0.0, post a comment here to let everyone know which of the items in the checklist above you'd like to work on. Then get to work on your PR!

We also need help reviewing new PRs and the backlog of Bootstrap v4 PRs that will need to get merged into master. If you've contributed to this project in the past and would like to help, send an email to the address on my GitHub profile. So far @lcreid, @donv, and @desheikh have all stepped up to help review PRs; thank you so much for your support!

lcreid commented 6 years ago

This sounds good, @mattbrictson. I will create a CONTRIBUTING.md. Initially I'll just move the existing "Contributing" section from the README.md to CONTRIBUTING.md.

GBH commented 6 years ago

The biggest problem is to remove all the old crud is not needed in rails 5+ and bootstrap4. A lot of helpers need to be reworked.

I'd add more to the list:

It's a buttloads of work. I actually had made form builder gem for bootstrap 2 and needed to rewrite it for bootstrap 3. Thankfully I found this gem.

lcreid commented 6 years ago

I did PR #346 to add form_with support without taking out form_for. It requires the near-duplication of a lot of test cases, but I think it's necessary to support both. Supporting form_for and form_with allows people to convert applications to use form_with one form at a time, which is less risky for them.

I'd support using Rubocop on the project at some point. There are a lot of long lines in this gem, so we might consider relaxing Rubocop's out-of-the-box line length restriction. I'd put most of the other items as higher priority than Rubocop right now.

mattbrictson commented 6 years ago

Agreed, I'd love to add RuboCop. I think we should merge the big outstanding PRs first, though, because if we apply a lot of RuboCop corrections that will result in a lot of merge conflicts I think.

desheikh commented 6 years ago

@mattbrictson, can you create a 4.0.0 milestone and tag the relevant PRs?

mattbrictson commented 6 years ago

can you create a 4.0.0 milestone and tag the relevant PRs?

Great idea! I'll set that up.

mattbrictson commented 6 years ago

I think it is important to document the migration path for users that will be upgrading from 2.7.0 to 4.0.0. I've added that to the task list.

mattbrictson commented 6 years ago

@GBH I like your idea of getting rid of Appraisal. It does seem to make things more complicated. Would you able to submit a PR for that? I think this could go hand-in-hand with removing Ruby < 2.3 and Rails < 5.0 in the Travis matrix.

GBH commented 6 years ago

@mattbrictson I can do that. Also, how do you feel about bumping requirements to Rails 5.1+. This way we only need to handle form_with going forward. Considering that new projects with Bootstrap 4 are likely to be only Rails 5.2+ anyways.

lcreid commented 6 years ago

I think we should try to support some range of Rails versions, because this allows people to move existing applications to a newer version of bootstrap_form without having to upgrade everything else. I think we'll see more adoption of bootstrap_form v 4.x if we support it on as many recent versions of Rails as is reasonable.

For example, I have a Rails 5.0 application that's using bootstrap_form from the bootstrap-4 branch. I'm going to switch to the official bootstrap_form 4.0 gem as soon as it's available, as long as it supports Rails 5.0. Judging by the number of forks, and PRs on the bootstrap-4 branch, I suspect I'm not the only one.

I'm sure it took some work to get appraisal set up originally, but now that it's working I think it's worth keeping.

mattbrictson commented 6 years ago

I appreciate the effort it took to set up Appraisal, but most of the benefits are already provided by Travis. I think we can remove it, as long as our CONTRIBUTING document explains how to test against multiple Rails versions locally.

I agree that we should remove 5.0 support eventually, but not for this 4.0.0 release. Personally, I have 5.1 apps that still use form_for and I would appreciate an easy migration path from form_for to form_with.

GBH commented 6 years ago

@mattbrictson when upgrading gem from 2.7 to 4.0 I don't think it's unreasonable to change bootstrap_form @shoe to bootstrap_form model: @shoe. Bigger issue is not supporting Rails 5.0. Alternatively you can try to support both form_for and form_with. That is a bit annoying though as you can't just merge those together (the reason why form_with is a thing). And you have to support form_with because Rails 5.1 and 5.2 users would expect a drop-in replacement, right?

mattbrictson commented 6 years ago

@GBH my preference is to support both form_for and form_with if it is not overly complicated. There are existing PRs e.g. #354 that suggest this is fairly straightforward. Once we get into reviewing/merging those PRs and it looks like supporting both is going to be a problem, then I'll reconsider this approach.

GBH commented 6 years ago

Fair enough 👍

lcreid commented 6 years ago

Doh! I get it. Travis will cover the versions. Makes sense.

lcreid commented 6 years ago

I submitted PR #346 for form_with and I can attest that the code for supporting both form_for and form_with isn't hard and is worth doing. There's a lot of near-duplication in the test code, but eventually that will go away, assuming Rails deprecates and then removes form_for.

ehutzelman commented 6 years ago

I just submitted PR #367 with a fix for a breaking change they made for v4-beta3 with regards to input group addons.

lcreid commented 6 years ago

I haven't done a thorough review of the outstanding PRs yet, but I think it's safe to say that there are some that overlap, in whole or in part. We're going to have to pick and choose the PRs we merge. It's frustrating to do the work for a PR and then see it go nowhere. Is there anything we can do for the people whose PRs don't get used, other than a big "thanks, anyway" in a comment?

mattbrictson commented 6 years ago

@lcreid You're doing a great job reviewing the PRs, thanks! In terms of picking PRs to merge, most of them will have conflicts and in extreme cases may need to be recreated from scratch against master. That takes time so I would just make sure to give the authors a chance to respond. I would wait maybe a week or so because people are busy and may not have time to write back immediately. I don't want to see someone's contribution discarded because we suddenly got impatient after making them wait for months.

For those PRs that we unfortunately have to decline I think a honest "thanks" will be fine, and point out that they can still help the project immensely if they would like to take the master branch for a spin and point out any problems they find.

Ultimately I thinking closing PRs is better than letting them sit open in limbo indefinitely, even if the conversation is a bit awkward.

CyberDeck commented 6 years ago

I just added the PR #370 to address the form validation issues (form-control-label, has-danger, form-control-feedback).

volkerholloh commented 6 years ago

@mattbrictson Great to have you back!

jeffblake commented 6 years ago

Aliasing for with and without is weird and unnecessary and should be removed. Can do the same thing with fields model: @foo, builder: ActionView::Helpers::FormBuilder do if you want to drop back to default builder.

This is not a good solution for nested fields because you lose the input[name] hierarchy.

Perhaps something like this instead, passing in super: true to the builder:

      define_method(method_name) do |name, options = {}|
        return super(name, options) if options[:super]
        form_group_builder(name, options) do
          prepend_and_append_input(options) do
            super name, options
          end
        end
      end
mattbrictson commented 6 years ago

@jeffblake thanks for bring that to my attention. Right now I do not plan on removing with_ and without_ as part of the 4.0.0 release. Yes, it would be nice to drop it at the same time we are making other breaking changes; it would certainly clean up the codebase and narrow the public API, both of which are compelling reasons. However if, like you said, this results in loss of functionality without a clean workaround, then it seems we should keep it around for the time being.

@GBH @jeffblake could you open a separate issue if you feel this warrants further discussion? I'd like to keep this thread focused on what remains to be done for Bootstrap v4 compatibility. Thanks!

GBH commented 6 years ago

I'm going to confirm how fields_for and fields_with behave. But at at the very least I'm 100% with @jeffblake for replacing methods chains with option that can be passed into a helper. Like builder: :default for example.

Going to 4.0.0 this is exactly right time to do API changes.

jeffblake commented 6 years ago

Sounds good! Agreed w/ GBH, now is a great time to nuke all those alias chains. Another minor reason to access the default builder via an option, rather than a method, is code editors (Textmate for me), highlight default rails methods nicely. And of course you can axe hundreds of lines of code, and implement the new way in one line of code, all while maintaining the same functionality.

A couple issues I noticed w/ Bootstrap 4:

mattbrictson commented 6 years ago

@jeffblake Thanks for your feedback and the suggestions. Regarding the offset classes, we have an outstanding PR #338 that address that problem, but it is stalled as I think the original author has moved on. Would be willing to submit a PR for it?

mattbrictson commented 6 years ago

check boxes and radios have changed. label no longer wraps the input

@jeffblake thanks for calling this out. We're tracking it in issue #395.

lcreid commented 6 years ago

I have an application in development that was using my fork of bootstrap_form with Bootstrap 4 and form_with support. Tonight I switched it to use bootstrap-ruby/bootstrap_form master. All but two Capybara tests passed, and I have reason to believe that the failures are Poltergeist-related, rather than bootstrap_form-related. Overall, the pages look much better than they did when using my fork. That's thanks to all the work others have done to bring bootstrap_form in line with the release version of Bootstrap 4. Well done, everyone!

lcreid commented 6 years ago

How extensive do you envision the UPGRADE-4.0.md document to be? I assume we would refer people to the Bootstrap documentation where applicable, rather than repeating it in our own documentation? So would it be strictly how to change the the user's Gemfile, and a reminder to change however they get Bootstrap?

I could make a try at the UPGRADE-4.0.md, but if I do I'd like to do it as I upgrade an app -- I have one that I've been meaning to upgrade to Bootstrap 4. However, it will take me a week or two at least to write the upgrade doc if I do that, because the app makes extensive use of Bootstrap 3 panels, which I'll have to change in many places.

Let me know if you want me to take a shot at the upgrade doc.

mattbrictson commented 6 years ago

@lcreid I don't think we need to explain the v3 to v4 upgrade process, as the Bootstrap documentation itself can cover that. For the UPGRADE-4.0.md doc, I am thinking more along the lines of other breaking changes. Right now we have these:

If someone depends on these removed features, what should they do so they can successfully upgrade? I personally have not used any of those three, so I am not sure what would actually be involved.

If we don't have good answers for that, then we probably don't have enough to write about to warrant a separate UPGRADE-4.0.md doc.

cseelus commented 6 years ago

We're using v4 already in production for a few weeks now, works perfectly.

Would it be possible to (beta-)release the status quo on Rubygems? Let me know if I can help somewhere.

lcreid commented 6 years ago

Thanks for the feedback. It's nice to hear from users when things are working for them!

We're about to release the alpha on Rubygems. We're just working on some documentation to make it easier for newbies like me to help. Stay tuned.

mattbrictson commented 6 years ago

bootstrap_form 4.0.0.alpha1 has been released! 🎉

https://rubygems.org/gems/bootstrap_form/versions/4.0.0.alpha1 https://github.com/bootstrap-ruby/bootstrap_form/releases/tag/v4.0.0.alpha1