-
-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in 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
.