search

sgruhier / foundation_rails_helper

Rails Helper for Zurb Fondation framework
MIT License
152 stars 84 forks source link

Update to Foundation 6 #126

Closed dsandstrom closed 8 years ago

dsandstrom commented 8 years ago

Foundation 6 is out so we need to make this gem compatible.

Fixes #121

TODO

dsandstrom commented 8 years ago

@dgmstuart I added a PR for the merge. Feel free to provide feedback or add TODOs.

dgmstuart commented 8 years ago

Yes, Foundation 6 should definitely be a major version bump, since it will involve breaking changes

I feel like we should continue the convention of only supporting the latest Rails and Foundation versions, and having unsupported branches for older versions. Migrating an existing project to 6 might be painful because there isn't a migration guide yet, but I feel like the trade-off is worth it.

I see the Rails 5 stuff is rolled into here as well: I guess it's just coincidence that foundation 6 and rails 5 are happening at (very) roughly the same time, but I guess it would make sense to have one supported branch (Foundation 6/Rails 5) and one new legacy branch (Foundation 5/Rails 4.2) - how do you feel about that?

On the other hand, is rolling in Rails 5 raising the barrier to entry of an upgrade by saying "you can't use Foundation 6 unless you also upgrade Rails"?

Am I right in thinking that there's no reliable way for the gem to complain if it's not using the expected version of Foundation?

dsandstrom commented 8 years ago

Thanks for the feedback.

I reverted anything that had to do with Rails 5 before merging into this foundation-6 branch. However, if another PR gets submitted (that we approve), I will merge it. Yet, I'd rather wait until we have an official release.

I think we can support Rails ~>4.1 and 5, for the time being. I'm not currently using 5 on any projects, so I can't say for certain. If it comes down to it, we can add a F6/Rails4 branch, but hopefully it's not needed.

I was looking for a migration guide, also. Luckily, I have a project that is still in development so I could test out F6 on it. The upgrade wasn't too bad.

dgmstuart commented 8 years ago

Rebased and squashed one of the 'fix' commits.

Most of the commits went away, since they were either back-and-forth changes to the gemspec, or were already on master.

dgmstuart commented 8 years ago

Regarding ruby versions, it's unclear what the foundation-rails gem itself supports:

I've raised an issue asking what they support: https://github.com/zurb/foundation-rails/issues/199

dsandstrom commented 8 years ago

I'll look into cherry picking your #142 commits into this branch.

dsandstrom commented 8 years ago

Ahhh, no deprecation warnings. Feels so nice.

dgmstuart commented 8 years ago

@dsandstrom Would it be cleaner to rebase this branch onto master rather than cherrypicking?

dsandstrom commented 8 years ago

Yes, I just didn't want to deal with conflicts.

dgmstuart commented 8 years ago

@dsandstrom. There aren't any conflicts - I wouldn't have expected there to be any.

I'll force-push the rebased branch now.

dgmstuart commented 8 years ago

Fear not the rebase. The rebase is your friend.

dgmstuart commented 8 years ago

Rebased again

dsandstrom commented 8 years ago

A man with no fears.

dgmstuart commented 8 years ago

Ok, I've pushed v2.0.0 as the last foundation-5 version and created the foundation-5 branch there - I think this is ready to become Master.

@dsandstrom is it ready to be released as 3.0.0, or is there more to do? Maybe better to get that last PR merged first.

dsandstrom commented 8 years ago

Since the API is still changing, we shouldn't release 3.0.0, but we should merge this into master. If we want to release 3.0.0.beta1, it wouldn't hurt.

dgmstuart commented 8 years ago

@dsandstrom cool - I wonder if there's value in keeping the foundation-6 branch around?

dsandstrom commented 8 years ago

No

dsandstrom commented 8 years ago

Well, #152 is a PR on foundation-6. That should be changed and rebased on master. It should be a easier rebase.

dgmstuart commented 8 years ago

@dsandstrom On second thoughts I'm nuking the foundation-6 branch: it'd be annoying if we got more PRs on it. Master is the branch for Foundation 6 - nice and simeple

dgmstuart commented 8 years ago

@dsandstrom high-five - I think everything is where it should be, and we have zero PRs ✋