-
-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Use ruby2_keywords
for the moment
#205
Conversation
ea28993
to
9bbb771
Compare
@javierjulio After a second look, I believe the most conservative option is to use |
Without this gem, current activeadmin version becomes incompatible with the master branch of `arbre`. It shouldn't be an issue as long as we update activeadmin's code to use keyword arguments everywhere, set a minimum dependency on the next `arbre` and release both gems at the same time, but I think it's easier to be conservative.
9bbb771
to
26a4391
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem at all. I'm just not sure I understand how its incompatible. Either way though this approach is fine too, thanks! 🙏🏻
I didn't spend time understanding it either, I just run activeadmin test suite against current arbre's master and stuff failed like this:
With this PR nothing fails. I also noticed that everything is fine with activeadmin/activeadmin#6237, but still I don't want to leave any combination where things could break for users. |
Ha even then though that output says a lot. Yes, I agree, with the gem at least we know for sure it's compatible. Thanks! Let's do it. 👍🏻 |
Without this gem, current activeadmin version becomes incompatible with the master branch of `arbre`. It shouldn't be an issue as long as we update activeadmin's code to use keyword arguments everywhere, set a minimum dependency on the next `arbre` and release both gems at the same time, but I think it's easier to be conservative.
Without this gem, current activeadmin version becomes incompatible with the master branch of `arbre`. It shouldn't be an issue as long as we update activeadmin's code to use keyword arguments everywhere, set a minimum dependency on the next `arbre` and release both gems at the same time, but I think it's easier to be conservative.
Without this gem, current activeadmin version becomes incompatible with the master branch of `arbre`. It shouldn't be an issue as long as we update activeadmin's code to use keyword arguments everywhere, set a minimum dependency on the next `arbre` and release both gems at the same time, but I think it's easier to be conservative.
* render_in: Use `ruby2_keywords` for the moment (activeadmin#205) Deprecate Element#to_s and #to_str Add Document#render_in Use ActiveSupport::SafeBuffer instead of StringIO. require 'timeout' Use render_in(self) to build context cached_html. Add FieldsForProxy#render_in Deprecate using :to_s for rendering. Test using output_buffer from html tags matches #to_s. Add input and output variables to html rendering specs. Add Context#output_buffer, Element#render_in, ElementCollection#render_in, TextNode#render_in and Tag#render_in.
Without this gem, current
activeadmin
version becomes incompatible with the master branch ofarbre
. It shouldn't be an issue as long as we updateactiveadmin
's code to use keyword arguments everywhere, set a minimum dependency on the nextarbre
and release both gems at the same time, but I think it's easier to be conservative.So let's use
ruby2_keywords
.