Skip to content

Code cleanup #2369

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

Merged
merged 4 commits into from
Jan 3, 2020
Merged

Code cleanup #2369

merged 4 commits into from
Jan 3, 2020

Conversation

wasifhossain
Copy link
Member

No description provided.

@@ -73,7 +73,7 @@ def self.serializer_lookup_chain_for(klass, namespace = nil)
# Used to cache serializer name => serializer class
# when looked up by Serializer.get_serializer_for.
def self.serializers_cache
@serializers_cache ||= ThreadSafe::Cache.new
@serializers_cache ||= Concurrent::Map.new
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kind of odd. I'm not sure where the dependency is coming from. I know later versions of Rails introduce concurrent-ruby. I guess thread_safe was coming from active_support?

Copy link
Member Author

@wasifhossain wasifhossain Jan 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thread_safe is needed by 2 different sources:

  • active_support (4.2.11.1)
  • grape (0.19.0) (through virtus)

def link(name, value = nil)
options[:links][name] = block_given? ? Proc.new : value
def link(name, value = nil, &block)
options[:links][name] = block_given? ? block : value
Copy link
Member

@bf4 bf4 Jan 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm installing 2.7 locally so I can figure out what you're seeing. As I wrote in the other PR, this change doesn't make sense to me

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, this makes more sense

lib/active_model/serializer/reflection.rb:91: warning: Capturing the given block using Proc.new is
deprecated; use &block instead

Copy link
Member

@bf4 bf4 Jan 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bf4 bf4 merged commit 64c7fee into 0-10-stable Jan 3, 2020
@bf4 bf4 deleted the ruby-2-7-prep branch January 3, 2020 02:54
@bf4 bf4 mentioned this pull request Jan 3, 2020
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