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.

Sign up for GitHub

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

Make more class attributes inheritable #1255

Merged
merged 1 commit into from
Oct 9, 2015
Merged

Make more class attributes inheritable #1255

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions lib/active_model/serializer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ class Serializer
)
/x

class << self
attr_accessor :_attributes
attr_accessor :_attributes_keys
end

with_options instance_writer: false, instance_reader: false do |serializer|
class_attribute :_type, instance_reader: true
class_attribute :_attributes
self._attributes ||= []
class_attribute :_attributes_keys
Copy link
Contributor

Choose a reason for hiding this comment

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

style-wise, should the assignments be right next to eachother, rather than separated by a class_attribute definition?

Copy link
Member Author

Choose a reason for hiding this comment

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

no idea

Copy link
Contributor

Choose a reason for hiding this comment

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

well, I guess it's my personal preference to group that kind of stuff together. makes it slightly easier to skim, imo

Copy link
Member Author

Choose a reason for hiding this comment

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

Ex or pr?

B mobile phone

On Oct 7, 2015, at 6:36 AM, L. Preston Sego III notifications@github.com wrote:

In lib/active_model/serializer.rb:

 with_options instance_writer: false, instance_reader: false do |serializer|

  •  class_attribute :_type, instance_reader: true

  •  class_attribute :_attributes

  •  self._attributes ||= []

  •  class_attribute :_attributes_keys
    well, I guess it's my personal preference to group that kind of stuff together. makes it slightly easier to skim, imo

—
Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

huh?

Copy link
Member Author

Choose a reason for hiding this comment

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

Meant example code of what you prefer.

B mobile phone

On Oct 8, 2015, at 2:53 PM, L. Preston Sego III notifications@github.com wrote:

In lib/active_model/serializer.rb:

 with_options instance_writer: false, instance_reader: false do |serializer|

  •  class_attribute :_type, instance_reader: true

  •  class_attribute :_attributes

  •  self._attributes ||= []

  •  class_attribute :_attributes_keys
    huh?

—
Reply to this email directly or view it on GitHub.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bf4 I think he prefers having all the class_attribute grouped together, and then the stuff ||= stuff. I personally do not have an opinion on the matter.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think i like either but inclined to this. Good to merge?

B mobile phone

On Oct 8, 2015, at 6:25 PM, Lucas Hosseini notifications@github.com wrote:

In lib/active_model/serializer.rb:

 with_options instance_writer: false, instance_reader: false do |serializer|

  •  class_attribute :_type, instance_reader: true

  •  class_attribute :_attributes

  •  self._attributes ||= []

  •  class_attribute :_attributes_keys
    @bf4 I think he prefers having all the class_attribute grouped together, and then the stuff ||= stuff. I personally do not have an opinion on the matter.

—
Reply to this email directly or view it on GitHub.

self._attributes_keys ||= {}
serializer.class_attribute :_cache
serializer.class_attribute :_fragmented
serializer.class_attribute :_cache_key
Expand All @@ -43,8 +43,8 @@ class << self
end

def self.inherited(base)
base._attributes = _attributes.try(:dup) || []
base._attributes_keys = _attributes_keys.try(:dup) || {}
base._attributes = _attributes.dup
base._attributes_keys = _attributes_keys.dup
base._cache_digest = digest_caller_file(caller.first)
super
end
Expand Down Expand Up @@ -125,7 +125,6 @@ def self.get_serializer_for(klass)
end

attr_accessor :object, :root, :scope
class_attribute :_type, instance_writer: false

def initialize(object, options = {})
self.object = object
Expand All @@ -149,10 +148,10 @@ def attributes
attributes = self.class._attributes.dup

attributes.each_with_object({}) do |name, hash|
unless self.class._fragmented
hash[name] = send(name)
else
if self.class._fragmented
hash[name] = self.class._fragmented.public_send(name)
else
hash[name] = send(name)
end
end
end
Expand Down