Skip to content
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

Merged
merged 1 commit into from
May 21, 2020
Merged

Conversation

deivid-rodriguez
Copy link
Member

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.

So let's use ruby2_keywords.

@deivid-rodriguez
Copy link
Member Author

@javierjulio After a second look, I believe the most conservative option is to use ruby2_keywords after all.

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.
Copy link
Member

@javierjulio javierjulio left a 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! 🙏🏻

@deivid-rodriguez
Copy link
Member Author

I didn't spend time understanding it either, I just run activeadmin test suite against current arbre's master and stuff failed like this:

Failures:

  1) ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path without belongs_to works with no parent
     Failure/Error: li link_to(*args)
     
     ActionView::Template::Error:
       unknown keyword: :class
     # /home/deivid/Code/arbre/lib/arbre/element.rb:182:in `method_missing'
     # ./lib/active_admin/views/components/dropdown_menu.rb:40:in `block in item'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # ./lib/active_admin/views/components/dropdown_menu.rb:39:in `item'
     # /home/deivid/Code/arbre/lib/arbre/element.rb:178:in `method_missing'
     # ./lib/active_admin/batch_actions/views/batch_action_selector.rb:44:in `block (2 levels) in build_drop_down'
     # ./lib/active_admin/batch_actions/views/batch_action_selector.rb:30:in `each'
     # ./lib/active_admin/batch_actions/views/batch_action_selector.rb:30:in `block in build_drop_down'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:14:in `dropdown_menu'
     # ./lib/active_admin/batch_actions/views/batch_action_selector.rb:27:in `build_drop_down'
     # ./lib/active_admin/batch_actions/views/batch_action_selector.rb:14:in `build'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:30:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # ./lib/active_admin/views/pages/index.rb:78:in `build_batch_actions_selector'
     # ./lib/active_admin/views/pages/index.rb:64:in `block in build_table_tools'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:14:in `div'
     # ./lib/active_admin/views/pages/index.rb:63:in `build_table_tools'
     # ./lib/active_admin/views/pages/index.rb:27:in `block in main_content'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:14:in `batch_action_form'
     # ./lib/active_admin/views/pages/index.rb:36:in `wrap_with_batch_action_form'
     # ./lib/active_admin/views/pages/index.rb:26:in `main_content'
     # ./lib/active_admin/views/pages/base.rb:101:in `block (2 levels) in build_main_content_wrapper'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:14:in `div'
     # ./lib/active_admin/views/pages/base.rb:100:in `block in build_main_content_wrapper'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:14:in `div'
     # ./lib/active_admin/views/pages/base.rb:99:in `build_main_content_wrapper'
     # ./lib/active_admin/views/pages/base.rb:83:in `block in build_page_content'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:14:in `div'
     # ./lib/active_admin/views/pages/base.rb:82:in `build_page_content'
     # ./lib/active_admin/views/pages/base.rb:59:in `block (2 levels) in build_page'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:31:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:14:in `div'
     # ./lib/active_admin/views/pages/base.rb:55:in `block in build_page'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:49:in `with_current_arbre_element'
     # ./lib/active_admin/views/pages/base.rb:54:in `build_page'
     # ./lib/active_admin/views/pages/base.rb:9:in `build'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:30:in `block in build_tag'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:102:in `with_current_arbre_element'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:26:in `build_tag'
     # /home/deivid/Code/arbre/lib/arbre/element/builder_methods.rb:39:in `insert_tag'
     # ./app/views/active_admin/resource/index.html.arb:2:in `block in __home_deivid__ode_activeadmin_app_views_active_admin_resource_index_html_arb__2351605089594022382_27860'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:45:in `instance_eval'
     # /home/deivid/Code/arbre/lib/arbre/context.rb:45:in `initialize'
     # ./app/views/active_admin/resource/index.html.arb:1:in `new'
     # ./app/views/active_admin/resource/index.html.arb:1:in `__home_deivid__ode_activeadmin_app_views_active_admin_resource_index_html_arb__2351605089594022382_27860'
     # ./lib/active_admin/resource_controller/streaming.rb:12:in `index'
     # ./spec/unit/resource_controller/polymorphic_routes_spec.rb:18:in `block (4 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # ArgumentError:
     #   unknown keyword: :class
     #   /home/deivid/Code/arbre/lib/arbre/element.rb:182:in `method_missing'

(... 27 others)


Finished in 11.25 seconds (files took 8.79 seconds to load)
179 examples, 28 failures

Failed examples:

rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:1:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path without belongs_to works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:2:1:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with belongs_to within the posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:2:1:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with belongs_to within the posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:2:2:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with belongs_to within the user_posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:2:2:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with belongs_to within the user_posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:3:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the user_posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:3:3] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the user_posts page works with a category as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:3:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the user_posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:2:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the category_posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:2:3] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the category_posts page works with a category as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:2:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the category_posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:1:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:1:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:2:3:1:3] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_path with multiple belongs_to (not fully supported yet) within the posts page works with a category as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:2:2:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with belongs_to within the user_posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:2:2:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with belongs_to within the user_posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:2:1:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with belongs_to within the posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:2:1:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with belongs_to within the posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:1:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url without belongs_to works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:1:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:1:3] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the posts page works with a category as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:1:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:2:3] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the category_posts page works with a category as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:2:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the category_posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:2:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the category_posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:3:1] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the user_posts page works with no parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:3:2] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the user_posts page works with a user as parent
rspec ./spec/unit/resource_controller/polymorphic_routes_spec.rb[1:1:3:3:3] # ActiveAdmin::ResourceController::PolymorphicRoutes polymorphic_url with multiple belongs_to (not fully supported yet) within the user_posts page works with a category as parent

Randomized with seed 14881

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.

@javierjulio
Copy link
Member

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. 👍🏻

@deivid-rodriguez deivid-rodriguez merged commit b546d7a into master May 21, 2020
@deivid-rodriguez deivid-rodriguez deleted the ruby2_keywords branch May 21, 2020 18:15
omitter added a commit to omitter/arbre that referenced this pull request Dec 23, 2021
varyonic pushed a commit to varyonic/arbo that referenced this pull request Nov 23, 2022
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.
varyonic pushed a commit to varyonic/arbo that referenced this pull request Nov 23, 2022
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.
varyonic pushed a commit to varyonic/arbo that referenced this pull request Dec 1, 2022
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.
varyonic added a commit to varyonic/arbo that referenced this pull request Dec 9, 2022
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants