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

Update active_model_serializers 0.9.7 to 0.10.10 #8154

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

pravi
Copy link
Contributor

@pravi pravi commented Sep 21, 2020

@pravi pravi marked this pull request as draft September 21, 2020 10:07
@pravi pravi force-pushed the update-active-model-serializers branch from 72ca1bf to 283a746 Compare September 21, 2020 10:49
@pravi
Copy link
Contributor Author

pravi commented Sep 21, 2020

@pravi
Copy link
Contributor Author

pravi commented Sep 22, 2020

some pointers rails-api/active_model_serializers#2028

@pravi pravi force-pushed the update-active-model-serializers branch from 283a746 to f725a63 Compare September 22, 2020 06:32
@@ -82,6 +82,6 @@ def pick_serializer_class
# NOTE: this matcher uses knowledge of AMS internals
RSpec::Matchers.define :serialize_each_with do |expected|
match do |actual|
actual.is_a?(ActiveModel::ArraySerializer) && actual.instance_variable_get("@each_serializer") == expected
actual.is_a?(ActiveModelSerializers::SerializableResource) && actual.instance_variable_get("@each_serializer") == expected

Choose a reason for hiding this comment

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

Layout/LineLength: Line is too long. [126/120]

@pravi pravi force-pushed the update-active-model-serializers branch from f725a63 to b2857ee Compare September 22, 2020 07:04
@@ -0,0 +1,166 @@
module AMS

Choose a reason for hiding this comment

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

Style/FrozenStringLiteralComment: Missing frozen string literal comment.

module AMS
module V09
class Serializer < ActiveModel::Serializer

Choose a reason for hiding this comment

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

Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body beginning.

module V09
class Serializer < ActiveModel::Serializer

def serializable_hash(adapter_options = nil,

Choose a reason for hiding this comment

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

  • Layout/IndentationWidth: Use 2 (not 3) spaces for indentation.
  • Layout/SpaceAroundEqualsInParameterDefault: Surrounding space detected in default value assignment.

class Serializer < ActiveModel::Serializer

def serializable_hash(adapter_options = nil,
options = {},

Choose a reason for hiding this comment

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

Layout/SpaceAroundEqualsInParameterDefault: Surrounding space detected in default value assignment.


def serializable_hash(adapter_options = nil,
options = {},
adapter_instance = self.class.serialization_adapter_instance)

Choose a reason for hiding this comment

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

Layout/SpaceAroundEqualsInParameterDefault: Surrounding space detected in default value assignment.

@included[json_key] ||= []

if serializer.respond_to?(:each)
serializer.each { |s| process_relationship(s, key) }

Choose a reason for hiding this comment

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

Layout/SpaceInsideBlockBraces: Space between { and | detected.

serializer.each { |s| process_relationship(s, key) }
return
end
return unless serializer && serializer.object

Choose a reason for hiding this comment

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

Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method.

end

def meta_key
instance_options.fetch(:meta_key, 'meta'.freeze)

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

def meta_key
instance_options.fetch(:meta_key, 'meta'.freeze)
end

Choose a reason for hiding this comment

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

Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body end.

end

end

Choose a reason for hiding this comment

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

Layout/EmptyLinesAroundModuleBody: Extra empty line detected at module body end.

@denschub
Copy link
Member

Could you please run tests and code style checks on your machine before pushing? Having the code review bot spam the same comments over and over again frequently ends in the bot being temporarily disabled, and that's quite annoying to deal with. :)

@jhass
Copy link
Member

jhass commented Apr 11, 2021

Not sure what the benefit of upgrading here is while not porting the code but using a compat layer. A proper port is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants