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

Add "defaults" DSL that helps to declare properties #138

Merged
merged 1 commit into from
Oct 31, 2015

Conversation

sadjow
Copy link

@sadjow sadjow commented May 26, 2015

Hey @apotonick , this is a proposal for these issues:

#136
#131
#43

The code is implemented. I'll add some documentation..

I'll write something to the links methods too.

  class SongRepresenter < ...
    defaults do
      property render_nil: true
    end
  end

or

  class SongRepresenter < ...
    defaults do
      property render_nil: true do |name|
       { as: name.to_s.camelize }
      end
    end
  end

or even with hashes ...

  class SongRepresenter < ...
    defaults property: { render_nil: true }
  end

@apotonick
Copy link
Member

Wow, thanks! I was thinking about that a lot last weekend. Cooool! 🎸

I am wondering what is the property good for? Couldn't we just do the following:

defaults render_nil: true do |name|
  { as: name.to_s.camelize }
end

@sadjow
Copy link
Author

sadjow commented May 26, 2015

Nice to hear from you @apotonick ,

You proposed that DSL at: #136 (comment) then, I made the property method thinking in implement the link method also.

But, if the link method is not necessary, I think that the proposed new one is even nicer. 💯

defaults render_nil: true do |name|
 { as: name.to_s.camelize }
end

It is also possible to work with the given specific options for the property.

defaults render_nil: true do |name, options|
  camelize_stype = options.fetch(:camelize_stype, :lower)
  { as: name.to_s.camelize(camelize_stype) }
end

property :title
property :author_name, camelize_stype: :upper # if the API you are consuming has no standard. 

# This is only a example. You could work with all options for the property.

@sadjow sadjow force-pushed the declarative-defaults branch 2 times, most recently from ae16bdd to b6f4031 Compare May 26, 2015 23:35
@sadjow
Copy link
Author

sadjow commented May 26, 2015

@apotonick The code was updated.

Defaults for options

Declaring default options for property, nested, collection and hash is
easy as:

defaults render_nil: true

You can also pass a block for dynamic options based on the property name or even
on the given specific property options.

defaults render_nil: true do |name|
  { as: name.to_s.camelize }
end
# example: using the given property options to
# decide the value of other option.

defaults render_nil: true do |name, options|
  {
    as: name.to_s.camelize,
    embedded: options.fetch(:hash, false)
  }
end

You can clear the defaults options using clear_defaults.

defaults render_nil: true

property :name # will render if nil

clear_defaults

property :description # will not render if nil

@sadjow
Copy link
Author

sadjow commented May 28, 2015

@apotonick, any thoughts?

@apotonick
Copy link
Member

It's great. Not sure if I like the clear_defaults, though. I have another idea of blocks.

@sadjow
Copy link
Author

sadjow commented May 28, 2015

Trying another approach ... (But, what's your idea?)

config :main, render_nil: true do |name|
  { as: name.to_s.camelize }
end

config :other, render_nil: true

default_config :main

# use the default config
property :song_title
property :song_attr2
property :song_attr3
property :song_attr4
property :song_attr5
# ...

with_config :other do
  # use the 'other' config
  property :name
  property :one
  property :two
  property :thre
end

property :foo, :other # use the 'other' config

without_config do
  property :description
  property :test
end

@sadjow
Copy link
Author

sadjow commented May 29, 2015

Hey @apotonick , I think the clear_defaults is not necessary, the specific properties could override the default values without removing them.

The best approach is to have only the:

defaults render_nil: true do |name|
  { as: name.to_s.camelize }
end

Generally the APIs have a standard in the field naming. I'll update the code.

@sadjow sadjow force-pushed the declarative-defaults branch from b6f4031 to fc208d0 Compare May 29, 2015 02:52
@sadjow
Copy link
Author

sadjow commented May 29, 2015

Code updated, commits smashed into one commit.

@sadjow
Copy link
Author

sadjow commented May 29, 2015

Docs preview:

Defaults

Declaring default options for property, nested, collection and hash is
easy as:

defaults render_nil: true

You can also pass a block for dynamic default options based on the property
name or on the property options.

defaults render_nil: true do |name|
  { as: name.to_s.camelize }
end
defaults render_nil: true do |name, options|
  {
    as: name.to_s.camelize,
    embedded: options.fetch(:hash, false)
  }
end

@apotonick
Copy link
Member

Loving your work! ❤️ I will move some things and change some other parts 😬 that way, I can use the defaults in Reform, Roar and Disposable instead of having to override ::property. That has been long planned in my head, but as always, you were quicker! 😆 🍺

@apotonick
Copy link
Member

@sadjow I decided to extract all schema logic from Representable into Declarative: https://github.com/apotonick/declarative. Can we re-apply your PR there?

@sadjow
Copy link
Author

sadjow commented Jun 25, 2015

Hey @apotonick I'll do that until tomorrow for sure.

@sadjow
Copy link
Author

sadjow commented Jun 25, 2015

🍻

@apotonick
Copy link
Member

Oh wait, I need to setup that repo first! 😬

@sadjow
Copy link
Author

sadjow commented Jun 25, 2015

😄

@apotonick
Copy link
Member

@sadjow Just letting you know: Thanks so much for your work!

I just merged it into the declarativee branch https://github.com/apotonick/representable/tree/declarativee which will become 2.4. I use your defaults feature now to simplify inheritance between decorators, modules, and features. Will blog soon and mention you. ❤️ 🍻

BTW, stay tuned for Representable 2.4, it's about 200% faster than 2.3 and wayyyy simpler implementation.

BTW2: My friend @noeliacabane (who did the excellent http://trailblazer.to site) is keen to move to SP for frontend design work. Any recommendations/connections for us?

@noeliacabane
Copy link

It can also be remote work =)

@sadjow
Copy link
Author

sadjow commented Oct 24, 2015

Nice to hear that @apotonick. Great! I'm really glad! Looking forward to see final representable 2.4 🚀 💚 ❤️ 🍻

@noeliacabane Unfortunately, I'm not living in SP anymore. My GitHub location was outdated. I'm in Natal RN, Brazil(far away from SP). I was working on a company that doesn't have frontend design jobs. Only backend(credit card processor). And I don't have much networking on design companies. Sorry, I can't help much on it. I lived only ~1.5 year in SP. But, about remote, I'll see Monday if my current company has a position for frontend design and I'll e-mail you! 😄

@apotonick apotonick merged commit fc208d0 into trailblazer:master Oct 31, 2015
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.

3 participants