Conversation
tagliala
added
the
do not merge
tagliala
force-pushed
the
feature/8904-devise5
branch
from
a818cc0 to
003028d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8936 +/- ##
=======================================
Coverage 99.08% 99.08%
=======================================
Files 139 139
Lines 4045 4045
=======================================
Hits 4008 4008
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tagliala
force-pushed
the
feature/8904-devise5
branch
2 times, most recently
from
4da4c52 to
4831fab
Compare
tagliala
removed
the
do not merge
|
@javierjulio do you prefer to bump the formtastic requirement in gemspec to v6? |
javierjulio
left a comment
javierjulio
left a comment
There was a problem hiding this comment.
@tagliala hmm I'm not sure. That depends if Formtastic v6 supports both Devise v4 and v5? Because of the update to support Devise v5, wouldn't we at least want to require Devise v5 as the minimum on our side? Do you think its necessary that we bump the minimum requirement for Formtastic to >= v6?
|
TL;DR: Keeping current gemspec dependencies should be fine
Formtastic v6 just drops some legacy rails and ruby versions. It also matches 3.2 / 7.2 that we have here Additionally, Formtastic v6's changelog mentions "compatibility with Devise 5", but that is a fix to allow rails proc validation Master branch now test Devise v4 against Formtastic v6, and everything is fine
This does make sense too, but probably this is not necessary and may cause friction. Devise has a lot of plugins, maybe not all ready for v5, so keeping v4 compatibility should not be a problem
Not necessary, because that will have an impact on the main application as well edit
|
tagliala
force-pushed
the
feature/8904-devise5
branch
2 times, most recently
from
20d12d8 to
ed7d787
Compare
|
Well done on getting everything updated here and in Formtastic 👏🏻 🫶 @tagliala |
|
Is this gonna be on the 3.x branch or just for 4.0+? |
|
We need this patch on 3.x branch if it possible |
javierjulio
left a comment
javierjulio
left a comment
There was a problem hiding this comment.
@tagliala since we have a dependency on formtastic, I'm thinking we should bump the minimum requirement to v6 since only that version supports devise v4 and v5, since that sounds like your main intention. Someone could update formtastic to v6 but still be on devise v4 and their integration would break since there is no link between the two.
tagliala
force-pushed
the
feature/8904-devise5
branch
2 times, most recently
from
e91237a to
6d7f3bd
Compare
Keep Devise minimum unchanged to avoid unnecessary breakage for downstream apps. Raising the required versions would force upgrades without a strict need, since Devise 4 remains supported. Formtastic instead is required to be >= 6. It matches ActiveAdmin's minimum Ruby requirement and is compatible with both Devise 4 and 5. CI, however, will run the test matrix only against Devise 5 to validate compatibility with current versions. Additionally, update bundle syntax in spec for v4 Close #8906
tagliala
force-pushed
the
feature/8904-devise5
branch
from
6d7f3bd to
f843791
Compare
javierjulio
changed the title
5 participants
Keep Devise minimum unchanged to avoid unnecessary breakage for
downstream apps. Raising the required versions would force upgrades
without a strict need, since Devise 4 remains supported.
Formtastic instead is required to be >= 6. It matches ActiveAdmin's
minimum Ruby requirement and is compatible with both Devise 4 and 5.
CI, however, will run the test matrix only against Devise 5 to validate
compatibility with current versions.
Additionally, update bundle syntax in spec for v4
Close #8906